feat: add footPrint & priorityRule VS CODE extension#201
Conversation
kev59420
force-pushed
the
addMetricVsCodeExtension
branch
from
f4ac48e to
cd28941
Compare
|
|
I've made changes to fix the two failed builds previously. |
| // See https://go.microsoft.com/fwlink/?LinkId=733558 | ||
| // for the documentation about the tasks.json format |
There was a problem hiding this comment.
issue: We cannot include comments this way in a JSON File
A raw option could be to add a "_comments" array property at the root of the json object with these 2 lines as string items (not recommended)
A better way would be to
- add a README.md file in the .vscode folder with such tips
- add a json schema link property
"$schema": "vscode://schemas/tasks"- this vscode schema is dynamic which is why this link is very local
- An official schema exists but in a typescript format: https://code.visualstudio.com/docs/reference/tasks-appendix
| // A launch configuration that compiles the extension and then opens it inside a new window | ||
| // Use IntelliSense to learn about possible attributes. | ||
| // Hover to view descriptions of existing attributes. | ||
| // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 |
There was a problem hiding this comment.
issue: We cannot include comments this way in a JSON File
A raw option could be to add a "_comments" array property at the root of the json object with these 2 lines as string items (not recommended)
A better way would be to
- add a README.md file in the .vscode folder with such tips
- add a json schema link property
"$schema": "vscode://schemas/launch"- this vscode schema is dynamic which is why this link is very local
| } | ||
|
|
||
| /** | ||
| * @param {any} priorityRuleData |
There was a problem hiding this comment.
issue: any is not a valid JSDoc type
|
|
||
| await this.#updateBranches(); | ||
| await this.updatePriorityRule(); | ||
| await this.updateFootPrint(); |
There was a problem hiding this comment.
niptick: be consistent with the code base
The updateBranches() is private (defined by the JS standard symbol #), so should be the updatePriorityRule() and updateFootPrint() methods
|
|
||
| await this.#updateBranches(); | ||
| await this.updatePriorityRule(); | ||
| await this.updateFootPrint(); |
There was a problem hiding this comment.
typo: updateFootPrintshould be updateFootprint
Footprint is a single word
There was a problem hiding this comment.
suggestion(non-blocking): configuration.service.mjs files should be unit tested
Non blocking only because there no vscode unit tests yet
But highly recommended
There was a problem hiding this comment.
suggestion(non-blocking): score.script.mjs files should be unit tested
Non blocking only because there no vscode unit tests yet
But highly recommended
There was a problem hiding this comment.
suggestion(non-blocking): score.view.mjs files should be unit tested
Non blocking only because there no vscode unit tests yet
But highly recommended
There was a problem hiding this comment.
suggestion(non-blocking): extension.mjs files should be unit tested
Non blocking only because there no vscode unit tests yet
But highly recommended
| return issues | ||
| } | ||
|
|
||
| export async function getIssueCount({ project, branch }) { |
There was a problem hiding this comment.
issue: this function should be unit tested
| this.#service.show(true); | ||
| const url = `${server}coding_rules?languages=${priorityRule.lang}&q=${priorityRule?.name.split(' ').join('+')}&open=java%3AS2187` | ||
| this.#service.postMessage({ type: 'updatePriorityRule', value: { | ||
| priorityRule: priorityRule, |
There was a problem hiding this comment.
niptick: the syntax could be simplified
{ priorityRule, url } is enough
| * @param {number} [config.lifetime] The expected "lifetime" of the servers in years (default: 4) | ||
| * @param {number} [config.energyDemand] The annual "Energy Demand" for each server (default: 1670 kWh/year for a DELL PowerEdge R640) | ||
| * @param {number} [config.carbonIntensity] The "Carbon Intensity" of the environment where the application is running (default: 0.336 kgCO2/kWh for North America) | ||
| * @returns |
There was a problem hiding this comment.
doc: the returned type is missing
| window.showErrorMessage(error.message) | ||
| } | ||
| this.#service.show(true); | ||
| this.#service.postMessage({ type: 'updateFootPrint', value: footPrint }); |
There was a problem hiding this comment.
issue: the value is sent as a number without any formating
It should indicate the unit of measurement and the number of digits should be controlled
4 participants
Uh oh!
There was an error while loading. Please reload this page.