- Notifications
You must be signed in to change notification settings - Fork 2k
ci(lint): use super-linter#1371
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ttshivers commented Oct 19, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
0c251c4 to 430e0b3CompareSimenB commented Oct 19, 2020
This is great! Huge +1. Would you be up for fixing the violations as well? 🙂 |
ttshivers commented Oct 19, 2020
Sure. I can start off with the md files. Unsure about whether the Dockerfile lint errors should be fixed. |
SimenB commented Oct 19, 2020
Don't see a reason why they shouldn't? /cc @nodejs/docker |
PeterDaveHello commented Oct 19, 2020
I vote -0 here, there are many details in super-linter we may need to control by ourselves, like the versions of the linters, the excluded rules of the linters, etc. |
ttshivers commented Oct 19, 2020
Some of the current errors: hadolint (docker): Unsure if some of these should be fixed or not like whether the alpine apk packages be pinned? shfmt: It looks like it prefers no space after the redirection. Errors are mostly all like this. -hash git 2> /dev/null ||{echo >&2 "git not found, exiting."}+hash git 2>/dev/null ||{echo >&2 "git not found, exiting."}js (standard). All semicolon or comma: markdownlint. Misc errors: |
nschonni commented Oct 19, 2020
It might help to add some problem matches so the specific lines are highlighted.
|
SimenB commented Oct 21, 2020
Sorta odd the "super" linter doesn't come with problem matchers |
6c21201 to 28ed739Comparenschonni commented Oct 22, 2020
Looks like it won't do more than 10 annotations for the non-changed files, but that's probably OK |
a28dd5b to 818c8f4Comparenschonni commented Oct 22, 2020
I'm thinking that for the |
ttshivers commented Oct 23, 2020
Yeah that is doable. I could more easily do this when my node port to |
PeterDaveHello commented Oct 23, 2020
We use shfmt as Also wondering, if super-linter updated a version of the linters which need our changes, but we don't have the effort to resolve it immediately, it could be a problem, as super-linter doesn't seem to be able to specify the version of the linters. |
ttshivers commented Oct 23, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Sure, I think I could make shfmt use those parameters. I think it follows the editorconfig: https://github.com/github/super-linter/blob/master/docs/disabling-linters.md#shfmt-config-file As for the versioning, the current actions (except for shfmt) aren't pinned atm. The new action pins super-linter so I wouldn't expect any non-major version increase to introduce any breaking changes. |
7b56b85 to 589edd1ComparePeterDaveHello commented Oct 23, 2020
If we use the same parameters, I think the shell scripts should be the same without any change? Looks like the indentation was still revised 🤔
Cool 👍 |
00f7976 to 2e23cbdCompareReplace all other linters with super-linter
SimenB commented Mar 10, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Can we revive this? And also set |
Replace all other linters with super-linter (docs here): https://github.com/github/super-linter
It has all the linters that the previous actions used.
Docs for disabling rules: https://github.com/github/super-linter/blob/master/docs/disabling-linters.md
Refs: #1368 (comment)