Skip to content

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Mar 28, 2025

@fahedouch @AkihiroSuda

This comes on top of #4048 of course.

#4048 changed almost nothing in code.

This here:

  • fixes a handful of code issues allowing us to re-enable rules
  • re-organize the golangci yml file to document rules and priorities to fix

For the first part that actively change code, each fix is a separate commit, so, it should be easy to just drop one if we do not agree that it was necessary:

  • revive redundant-import-alias
  • revive struct-tag
  • revive time-equal
  • revive unexported-naming
  • revive if-return
  • revive filename-format
  • revive import-alias-naming
  • static check QF1004
  • static check ST1005

For the second part, I did organize rules in groups:

  • P0: must fix ASAP
  • P1: not critical, but should fix
  • P2: nice to have
  • Disabled: we don't want them

I also added the number of occurrences for each failing rule, giving the reader a sense of how much of an effort that would be.

This of course is just my (informed) personal opinion on things, so, happy to change rules to go to different categories based on feedback, so, hit me up.

I also re-enabled gocritic (and disabled prealloc), but have not reviewed its ruleset yet.

@apostasie apostasie marked this pull request as ready for review March 28, 2025 18:57
@apostasie
Copy link
Contributor Author

CI failure is unrelated (#3513 )

@AkihiroSuda
Copy link
Member

Needs rebase

@AkihiroSuda AkihiroSuda added this to the v2.0.5 milestone Mar 30, 2025
@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Mar 30, 2025
- re-activate gocritic, disabling rules we do not pass
- disable prealloc (considered harmful)
- activate golines formatter
- comment and re-orgnize linters section

Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Note that QF1009 has been fixed by previous commits working on revive.

Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
@apostasie apostasie force-pushed the ci-golangci-v2-post branch from 9be2adc to 46c6af6 Compare March 30, 2025 16:22
@apostasie
Copy link
Contributor Author

Rebased.

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 14c10b1 into containerd:main Mar 30, 2025
30 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