fix(QF1003): Convert if/else-if chain to tagged switch#6037
Conversation
4bd441e to
9caac52
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
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:
|
| if node.Spec.Annotations.Name != "" { | ||
| return node.Spec.Annotations.Name, nil | ||
| if node.Spec.Name != "" { | ||
| return node.Spec.Name, nil |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I did this PR before you shared your hesitations with QF1008 . Would you like me to revert it ?
There was a problem hiding this comment.
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>
9caac52 to
54efe29
Compare
- What I did
Enable and fix QF1003 rule
- How I did it
Remove
-QF1003and rungolangci-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