New eG Innovations Plugin #11
Conversation
|
Adding a new plugin in squaredup |
shaswot77
added
new-plugin
…ations/squaredup-plugin-public into add-eginnovations-plugin
JSON parsing + other fixes
add error handling and better json handling
Updating from main
Syncup with main
…dd-eginnovations-plugin
…ations/squaredup-plugin-public into add-eginnovations-plugin if it merges an updated upstream into a topic branch.
There was a problem hiding this comment.
The requested changes are small ones and mostly around changes to our tooling. Otherwise there's nothing specific that concerns me!
In an advisory category:
You're importing objects but not (that I can see...) actually using them anywhere. You could drop these to simplify things (internally we don't import objects that we don't use without a good reason. If you have a good reason then please feel free to ignore me!).
You've got both axios and node-fetch as dependencies, but I don't think you're using axios at all. I'd suggest dropping that as a dependency, if only to make the bundle smaller and faster.
At the moment you're not using the report object in any capacity that I can see (just the log object). You can use report.error and report.warning to send back messages to the user in much the same way that you can use log.error to send messages to the log file. Something to think about for a future iteration maybe.
There seem to be a lot of label roles in your metadata, including on things like dates. We're normally a bit more targeted in our use of these roles so I'm not sure how this will behave (I think previously this could create excessively long tooltips on line graphs, but it's also been a long time since I last saw that issue so it may no longer be a thing).
| provides: { | ||
| type: 'string', | ||
| enum: ['health', 'templateData'] | ||
| enum: ['health', 'templateData','componentType'] |
There was a problem hiding this comment.
This is not a valid part of the schema. Can you please remove this?
| for (const app of page) { | ||
| addVertexForApp(context, app); | ||
| } |
There was a problem hiding this comment.
You're not awaiting this async function. There's nothing asynchronous in it so probably the easiest option is to just remove the async keyword on the function itself.
| // console.log( | ||
| // '=========================== Mermaid Source ===================================================================' | ||
| // ); | ||
| // console.log(source); | ||
| // console.log( | ||
| // '=========================== Copy/Paste the above into https://mermaid.live/edit ==============================' | ||
| // ); | ||
| // const mmUrl = `https://mermaid.ink/img/${Buffer.from(source).toString('base64')}`; | ||
| // console.log(`...or visit: ${mmUrl}`); | ||
|
|
||
| // const mmAnswer = await inquirer.prompt([ | ||
| // { | ||
| // name: 'showMmd', | ||
| // type: 'confirm', | ||
| // message: 'visit Mermaid URL now (opens in default browser)?' | ||
| // } | ||
| // ]); | ||
| // if (mmAnswer.showMmd) { | ||
| // open(mmUrl); | ||
| // } |
|
A failing unit test to look into @deepalisingh-r : `FAIL packages/@squaredup/unit-test/dataStreamDefaults.test.js |
5 participants
Tenant to Deploy to: eG Innovations Pvt Ltd
Description
Adding a new plugin for squaredup on premises application.
PR Creation Checklist:
breakingornon-breakingbug-fix,new-plugin,internalorenhancement