Skip to content

fix(QF1003): Convert if/else-if chain to tagged switch#6037

Merged
thaJeztah merged 1 commit into
docker:masterfrom
mmorel-35:staticcheck/quickfixes
Apr 28, 2025
Merged

fix(QF1003): Convert if/else-if chain to tagged switch#6037
thaJeztah merged 1 commit into
docker:masterfrom
mmorel-35:staticcheck/quickfixes

Conversation

@mmorel-35
Copy link
Copy Markdown
Contributor

@mmorel-35 mmorel-35 commented Apr 25, 2025

- What I did

Enable and fix QF1003 rule

- How I did it

Remove -QF1003 and run golangci-lint run --fix

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

No

@mmorel-35 mmorel-35 requested review from a team and silvin-lubecki as code owners April 25, 2025 18:42
@mmorel-35 mmorel-35 force-pushed the staticcheck/quickfixes branch from 4bd441e to 9caac52 Compare April 25, 2025 18:42
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.91%. Comparing base (c80675b) to head (54efe29).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6037   +/-   ##
=======================================
  Coverage   58.91%   58.91%           
=======================================
  Files         358      358           
  Lines       29988    29988           
=======================================
  Hits        17666    17666           
  Misses      11341    11341           
  Partials      981      981           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vvoland vvoland added this to the 28.2.0 milestone Apr 28, 2025
Copy link
Copy Markdown
Collaborator

@vvoland vvoland left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Comment thread cli/command/idresolver/idresolver.go Outdated
Comment on lines +39 to +40
if node.Spec.Annotations.Name != "" {
return node.Spec.Annotations.Name, nil
if node.Spec.Name != "" {
return node.Spec.Name, nil
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'm still not sure if we want to enable the QF1008 ("Omit embedded fields from selector expression") check. While in some cases it's OK to skip the implicit embedded reference, there's many cases where explicit may be preferred over implicit, and being explicit can (for some) help discover situations where a "wrapper" struct is passed, but only an embedded struct is needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did this PR before you shared your hesitations with QF1008 . Would you like me to revert it ?

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.

If it's not too much to ask, perhaps that's better. But keep the patch around, and feel free to open it as a separate (draft) PR for further discussion. Possibly there's some packages where it does make sense to use the implicit embedded types, just not something we can / should use as an enforced check.

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
@mmorel-35 mmorel-35 force-pushed the staticcheck/quickfixes branch from 9caac52 to 54efe29 Compare April 28, 2025 16:35
@mmorel-35 mmorel-35 changed the title fix: staticcheck quickfixes issues fix(QF1003): Convert if/else-if chain to tagged switch Apr 28, 2025
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah modified the milestones: 28.2.0, 28.1.2 Apr 28, 2025
@thaJeztah thaJeztah merged commit fb261fd into docker:master Apr 28, 2025
93 checks passed
@mmorel-35 mmorel-35 deleted the staticcheck/quickfixes branch April 28, 2025 19:22
@thaJeztah thaJeztah modified the milestones: 28.1.2, 28.2.0 May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants