docker-node icon indicating copy to clipboard operation
docker-node copied to clipboard

ci(lint): use super-linter

Open ttshivers opened this issue 3 years ago • 14 comments

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: https://github.com/nodejs/docker-node/pull/1368#issuecomment-709465574

ttshivers avatar Oct 19 '20 06:10 ttshivers

This is great! Huge +1. Would you be up for fixing the violations as well? 🙂

SimenB avatar Oct 19 '20 08:10 SimenB

Sure. I can start off with the md files. Unsure about whether the Dockerfile lint errors should be fixed.

ttshivers avatar Oct 19 '20 08:10 ttshivers

Don't see a reason why they shouldn't?

/cc @nodejs/docker

SimenB avatar Oct 19 '20 08:10 SimenB

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.

PeterDaveHello avatar Oct 19 '20 09:10 PeterDaveHello

Some of the current errors:

hadolint (docker):

/github/workspace/12/alpine3.12/Dockerfile:5 DL3003 Use WORKDIR to switch to a directory
/github/workspace/12/alpine3.12/Dockerfile:5 DL3018 Pin versions in apk add. Instead of `apk add <package>` use `apk add <package>=<version>`
/github/workspace/12/alpine3.12/Dockerfile:5 DL4006 Set the SHELL option -o pipefail before RUN with a pipe in it. If you are using /bin/sh in an alpine image or if your shell is symlinked to busybox then consider explicitly setting your SHELL to /bin/ash, or disable this check
/github/workspace/12/alpine3.12/Dockerfile:76 SC2043 This loop will only ever run once. Bad quoting or missing glob/expansion?

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:

/github/workspace/genMatrix.js:3:25: Extra semicolon.
/github/workspace/genMatrix.js:7:37: Unexpected trailing comma.

markdownlint. Misc errors:

/github/workspace/README.md:61:1 MD014/commands-show-output Dollar signs used before commands without showing output [Context: "$ docker build -t my-nodejs-ap..."]
/github/workspace/README.md:245 MD024/no-duplicate-heading/no-duplicate-header Multiple headings with the same content [Context: "#### Docker Working Group Memb..."]][0m
/github/workspace/docs/BestPractices.md:41 MD028/no-blanks-blockquote Blank line inside blockquote
/github/workspace/docs/BestPractices.md:86 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
/github/workspace/docs/BestPractices.md:173:149 MD034/no-bare-urls Bare URL used [Context: "https://github.com/docker/dock..."]][0m

ttshivers avatar Oct 19 '20 16:10 ttshivers

It might help to add some problem matches so the specific lines are highlighted.

  • If you use the "acitons/setup-node" it will register the one for ESLint
  • Markdownlint file https://github.com/dotnet/docs/blob/master/.github/workflows/markdownlint-problem-matcher.json and line to register it https://github.com/dotnet/docs/blob/713137dbeaa9d6782dd837cc88cfd83ab5aa2330/.github/workflows/markdownlint.yml#L30

nschonni avatar Oct 19 '20 16:10 nschonni

Sorta odd the "super" linter doesn't come with problem matchers

SimenB avatar Oct 21 '20 21:10 SimenB

Looks like it won't do more than 10 annotations for the non-changed files, but that's probably OK

nschonni avatar Oct 22 '20 03:10 nschonni

I'm thinking that for the This loop will only ever run once. Bad quoting or missing glob/expansion? for the Yarn keys. Maybe we should just flatten the loop, since there only appears to be a single signing key for them

nschonni avatar Oct 22 '20 13:10 nschonni

I'm thinking that for the This loop will only ever run once. Bad quoting or missing glob/expansion? for the Yarn keys. Maybe we should just flatten the loop, since there only appears to be a single signing key for them

Yeah that is doable. I could more easily do this when my node port to update.js is finished.

ttshivers avatar Oct 23 '20 03:10 ttshivers

We use shfmt as shfmt -sr -i 2 -l -w -ci ., so there are some different places here, I prefer the original parameters we're using right now, instead of fix/reformat those scripts.

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.

PeterDaveHello avatar Oct 23 '20 04:10 PeterDaveHello

We use shfmt as shfmt -sr -i 2 -l -w -ci ., so there are some different places here, I prefer the original parameters we're using right now, instead of fix/reformat those scripts.

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.

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.

ttshivers avatar Oct 23 '20 04:10 ttshivers

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

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 🤔

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.

Cool 👍

PeterDaveHello avatar Oct 23 '20 07:10 PeterDaveHello

Can we revive this? And also set JAVASCRIPT_DEFAULT_STYLE to prettier instead of standard. And the slim image

SimenB avatar Mar 10 '22 08:03 SimenB