-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DX-2726 Initial Development of Linter CLI #2
Conversation
ran npm init and setup ts package coded lint command with --save option logic to download remote ruleset with local fallback
Not needed since we are publishing to GH registry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whole lot going on for new package setup, nice work! Please review comments, especially on Testing.
tests/cli.test.js
Outdated
}); | ||
|
||
it("should run lint command", async () => { | ||
expect(typeof testLint("./tests/fixtures/testSpec.yaml")).toBe("string"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest adding additional validation inspecting the string for error related message and error counts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better test one good spec and one bad spec
Also set result to be an array of objects instead of a single object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like the refactoring for improved validations, in much better state than before! Good with current improvements, if we've time left in the Sprint we can look at opportunity for other incremental improvements.
No description provided.