Skip to content

Add linter to Canary and fix go 1.23 linting issues #3321

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

Merged
merged 4 commits into from
Aug 17, 2024

Conversation

apostasie
Copy link
Contributor

This adds a lint validation to the Canary target.
Currently done on windows (because of the go install on the host), but will move to an independent stage after cleaning up the go_version detection routine.

Once green, we should merge ASAP to help with #3320

@apostasie apostasie marked this pull request as draft August 17, 2024 17:37
@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch 4 times, most recently from fbacb7b to f56bb72 Compare August 17, 2024 18:01
@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch from f56bb72 to d712613 Compare August 17, 2024 18:05
@AkihiroSuda
Copy link
Member

Thanks, in the commit message could you mention the fixed errors such as SA4032: due to the file's build constraints, runtime.GOOS will never equal "windows" (staticcheck)?

The commits can be also split for the each of the SA%d fixes.

@AkihiroSuda
Copy link
Member

go 1.23 linting issues

IIUC this is the matter of golangci-lint rather than go itself?

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 17, 2024
@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch 2 times, most recently from c6b9ba3 to 7999201 Compare August 17, 2024 18:17
@apostasie
Copy link
Contributor Author

go 1.23 linting issues

IIUC this is the matter of golangci-lint rather than go itself?

I think it is a combination of both? (eg: golangci-lint with that version of go)

Anyhow, thanks for the comments.

Will integrate all and split in different commits.

Right now struggling with windows (as usual), will re-request review when done.

@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch from 7999201 to cc7d341 Compare August 17, 2024 18:20
@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch from cc7d341 to fd0f954 Compare August 17, 2024 18:23
@apostasie apostasie force-pushed the b-dev-3320-canary-lint branch from 51a6e9d to 068866e Compare August 17, 2024 18:34
@apostasie apostasie marked this pull request as ready for review August 17, 2024 18:39
@apostasie apostasie requested a review from AkihiroSuda August 17, 2024 18:39
@apostasie
Copy link
Contributor Author

@AkihiroSuda canary lint is green. All comments addressed.

@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Aug 17, 2024
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit ef6c80b into containerd:main Aug 17, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci e.g., CI failure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants