Conversation
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
|
Do you think we should put the notes you have in this PR above in any documentation we have? Sorry, I now see that you did this already. |
README.md
Outdated
| * [Apply Attestations](#apply-attestations) | ||
|
|
||
|
|
||
| ### [Get Help with Converts](#converts) |
There was a problem hiding this comment.
| ### [Get Help with Converts](#converts) | |
| ### [Get Help with Convert](#convert) |
To me it makes more sense without the s. We could also be even more clear and say:
Get Help with Convert Commands
README.md
Outdated
| ``` | ||
| [top](#usage) | ||
| ### Convert | ||
| ### Converts |
There was a problem hiding this comment.
| ### Converts | |
| ### Convert |
Alternatively:
Convert Commands
README.md
Outdated
|
|
||
| Want to Recommend or Help Develop a Converter? See [on how to get started](https://github.com/mitre/saf/wiki/How-to-recommend-development-of-a-mapper) 📰 | ||
|
|
||
| [top](#get-help-with-converts) |
There was a problem hiding this comment.
| [top](#get-help-with-converts) | |
| [top](#get-help-with-convert) |
| // for ESLint https://eslint.style/ | ||
| // | ||
| // "eslint-plugin-n" -> This package provides additional ESLint rules for Node.js | ||
| // Originally the these were provided by the eslint-plugin-node that it is |
There was a problem hiding this comment.
| // Originally the these were provided by the eslint-plugin-node that it is | |
| // Originally these were provided by the eslint-plugin-node which is |
| } | ||
|
|
||
| case 'xlsx': { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
There was a problem hiding this comment.
How did you know when to disable the no-explicit-any vs. change the type to unknown?
There was a problem hiding this comment.
If I was able to determine the type I made the changes, otherwise I left it at any and disabled the linting.
Basically I follow the rule, if I can test to null and knew what the side effects were I changed. Most of the any for emassser I know what types to use or knew enough to create an interface.
| for (const key in controls) { | ||
|
|
||
| for (const key in controls) { // skipcq: JS-0051 |
There was a problem hiding this comment.
Where do you reference what each number lint rule means?
There was a problem hiding this comment.
I tried to find out, but couldn't. I use DeepSource to tell me about the finding
| const hideNulls: boolean = conf.displayNulls !== 'true' | ||
| const showEpoch: boolean = (conf.displayDateTime === 'true') | ||
| const debugging: boolean = (conf.debugging === 'true') | ||
| const hideNulls = !conf.displayNulls |
There was a problem hiding this comment.
Does this still work because it seems like the previous check was checking against a string rather than boolean type?
There was a problem hiding this comment.
yes, this is correct, the configuration where these are set changed.
test/commands/attest/apply.test.ts
Outdated
| }; | ||
| expect(omitHDFChangingFields(output)).to.eql(omitHDFChangingFields(expected)) | ||
| assert.isEmpty(stderr) | ||
| //expect(stderr).to.be.empty |
There was a problem hiding this comment.
| //expect(stderr).to.be.empty |
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable array-bracket-newline */ | |||
| /* eslint-disable array-element-newline */ | |||
|
|
|||
There was a problem hiding this comment.
We should remove these newlines
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable array-bracket-newline */ | |||
| /* eslint-disable array-element-newline */ | |||
|
|
|||
There was a problem hiding this comment.
Should we remove these newlines?
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable array-bracket-newline */ | |||
| /* eslint-disable array-element-newline */ | |||
|
|
|||
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable array-bracket-newline */ | |||
| /* eslint-disable array-element-newline */ | |||
|
|
|||
| @@ -1,5 +1,5 @@ | |||
| /* eslint-disable array-bracket-newline */ | |||
| /* eslint-disable array-element-newline */ | |||
|
|
|||
There was a problem hiding this comment.
I will hold from commenting in each situation. Whenever we update to remove the extra newlines, we can do a sweep to check for all of them.
| expect(fs.lstatSync((`${tmpobj.name}/RHEL_7/my-report.md`)).isFile()).to.be.true // skipcq: JS-0354 | ||
| const isReportFile = fs.lstatSync((`${tmpobj.name}/RHEL_7/my-report.md`)).isFile() | ||
| assert.isTrue(isReportFile) | ||
| // expect(fs.lstatSync((`${tmpobj.name}/RHEL_7/my-report.md`)).isFile()).to.be.true |
There was a problem hiding this comment.
| // expect(fs.lstatSync((`${tmpobj.name}/RHEL_7/my-report.md`)).isFile()).to.be.true |
test/commands/generate/delta.test.ts
Outdated
| expect(fs.lstatSync((`${tmpobj.name}/delta.md`)).isFile()).to.be.true // skipcq: JS-0354 | ||
| const isFile = fs.lstatSync((`${tmpobj.name}/delta.md`)).isFile() | ||
| assert.isTrue(isFile) | ||
| // expect(fs.lstatSync((`${tmpobj.name}/delta.md`)).isFile()).to.be.true |
There was a problem hiding this comment.
| // expect(fs.lstatSync((`${tmpobj.name}/delta.md`)).isFile()).to.be.true |
| expect(text).to.not.be.null // skipcq: JS-0354 | ||
| } catch (error: any) { | ||
| expect(error.message).to.equal('Request failed with status code 404') | ||
| // expect(text).to.not.be.null // skipcq: JS-0354 |
There was a problem hiding this comment.
| // expect(text).to.not.be.null // skipcq: JS-0354 |
| expect(appDiv).to.not.be.null // skipcq: JS-0354 | ||
| } catch (error: any) { | ||
| expect(error.message).to.equal('Request failed with status code 404') | ||
| // expect(appDiv).to.not.be.null // skipcq: JS-0354 |
There was a problem hiding this comment.
| // expect(appDiv).to.not.be.null // skipcq: JS-0354 |
em-c-rod
left a comment
There was a problem hiding this comment.
I left some comments and questions. Thank you for your work on updating all of this linting.
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
Signed-off-by: George M Dias <GDIAS@MITRE.ORG>
|


anyanywith `unknown when possibleSummary of using
anyorunknownunknownfor better type safety and explicit checksanyif you truly need unchecked, flexible behaviorSummary of linting configuration
ESLint packages used:
eslint–> The core ESLint package. It is the base linter.NOTE: Styles have moved from eslint -> @stylistic/eslint-plugin (js and ts)
Built-in stylistic rules for JavaScript and Typescript
We are still using legacy config, so we need to install v3.x with
npm i -D @stylistic/eslint-plugin@3@stylistic/eslint-plugin-> Used for general stylistic rules, which can beapplied to both JavaScript and TypeScript.
See here for additional information on ESLint Stylistic Stylistic Formatting
for ESLint https://eslint.style/
eslint-plugin-n-> This package provides additional ESLint rules for Node.jsOriginally the these were provided by the eslint-plugin-node that it is
no longer maintained
@eslint/js–> This package provides core ESLint rules for JavaScript. It’snecessary to allow ESLint to enforce JavaScript best practices.
typescript-eslint–> Tooling which enables the use of TypeScript with ESLint.This package is the entrypoint to consume additional tooling with ESLint.
The package exports the following:
config -> A utility function for creating type-safe flat configs
configs -> Shared ESLint (flat) configs
parser -> A re-export of @typescript-eslint/parser
plugin -> A re-export of @typescript-eslint/eslint-plugin
Notes: We still need to install the parser and plugin separately
@typescript-eslint/parser–> This parser allows ESLint to understand TypeScriptsyntax. Without it, ESLint won’t be able to process TypeScript files correctly.
@typescript-eslint/eslint-plugin–> This ESLint plugin provides additionalTypeScript-specific rules.
Optional package used that are very useful for best practice:
eslint-plugin-unicorn–> A plugin that provides a collection of additionalrules that enforce best practices and modern JavaScript standards. It’s
optional but useful for enforcing stricter code quality.
eslint-formatter-stylish–> A package that provides a stylish formatter forESLint. It’s optional but useful for a more readable output.
May need to use the chai plugin in the future
npm install --save-dev eslint-plugin-chai-friendlyAnd configure it into plugins: { "chai-friendly": chaiPLugin }