Skip to content

hack: add linting for multiple combinations of build tags#4070

Merged
jedevc merged 1 commit intomoby:masterfrom
jedevc:enable-linter-for-nydus
Jul 27, 2023
Merged

hack: add linting for multiple combinations of build tags#4070
jedevc merged 1 commit intomoby:masterfrom
jedevc:enable-linter-for-nydus

Conversation

@jedevc
Copy link
Copy Markdown
Member

@jedevc jedevc commented Jul 27, 2023

See #4067 and #4066.

Previously, we weren't linting the nydus tag either locally or in CI, so it was possible for things to slip. This PR adds linting for the nydus tag, and allows to easily extend this in the future.

We can't just add new build-tags to the golangci-lint config, since this tests only the combination of all of the provided tags. For the instance of nydus, we need to test with and without the build tag.

To do this, we lift the build tag logic into bake, and use the new matrix feature (though, note we can't actually run the golangci lint stages in parallel since 1. they all try and use all the CPU cores available, and 2. they play fun games when running at the same time that gives slightly bizarre and difficult to reproduce lint errors).

We can't just add new build-tags, since this tests only the combination
of all of the provided tags.  For the instance of nydus, we need to test
with *and* without the build tag.

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc requested a review from crazy-max July 27, 2023 09:11
Comment on lines +23 to +25
FROM scratch
COPY --link --from=golangci-lint /golangci-lint.done /
COPY --link --from=yamllint /yamllint.done /
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is to keep backward compat with previous behavior that had a single stage?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly yeah, we can remove this at some point later if we want though.

@jedevc jedevc merged commit 054daab into moby:master Jul 27, 2023
@jedevc jedevc deleted the enable-linter-for-nydus branch July 27, 2023 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants