Conversation
Add the modernize linter to .golangci.yml with only the bloop analyzer enabled. All other analyzers are disabled for now and will be enabled incrementally in future commits. The bloop analyzer fixes loop variable capture issues that can cause bugs when loop variables are captured by closures or goroutines. This issue was already addressed in the codebase by PR #17491 which converted benchmarks to use b.Loop() (Go 1.24+). Running golangci-lint --fix found 0 issues, confirming the codebase is already clean for loop variable capture problems. Reference commits: - PR #17491: Use b.Loop() for benchmarks (6a5b6a4) - Fix false positive in BenchmarkSliceRead (ad227d0f25) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Contributor
|
Images are ready for the commit at 4850eb9. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19520 +/- ##
==========================================
- Coverage 49.26% 49.23% -0.04%
==========================================
Files 2726 2727 +1
Lines 205628 205764 +136
==========================================
+ Hits 101307 101312 +5
- Misses 96783 96914 +131
Partials 7538 7538
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace misleading `omitempty` tags with `omitzero` on nested struct and time.Time fields where `omitempty` has no effect. Changes: - pkg/auth/authproviders/iap: googleClaims field - pkg/auth/authproviders/registry: time.Time expiry field - pkg/auth/authproviders/openshift: k8sapi.ObjectMeta field The `omitempty` tag doesn't work on: 1. Nested structs (not pointers) - always serialized even when empty 2. time.Time - zero value (Unix epoch) is valid, not considered "empty" With `omitzero`, these fields are now properly omitted when zero-valued, which is the intended behavior and improves API responses. Detected by: modernize/omitzero analyzer Applied manually after reviewing behavior change impact. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Using
json:"...,omitzero"introduces a non-standard struct tag thatencoding/jsondoes not understand; if these types are marshalled with the standard library, this will change or break JSON behavior—consider either removingomitemptyon nested structs or using pointers instead of introducingomitzeroin the tag. - If the goal is to enforce only the
omitzerocheck frommodernize, it might be clearer to enablemodernizeglobally and then explicitly disable it per-directory or via//nolintwhere needed, rather than disabling nearly all of its analyzers in the config.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `json:"...,omitzero"` introduces a non-standard struct tag that `encoding/json` does not understand; if these types are marshalled with the standard library, this will change or break JSON behavior—consider either removing `omitempty` on nested structs or using pointers instead of introducing `omitzero` in the tag.
- If the goal is to enforce only the `omitzero` check from `modernize`, it might be clearer to enable `modernize` globally and then explicitly disable it per-directory or via `//nolint` where needed, rather than disabling nearly all of its analyzers in the config.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
rhybrillou
approved these changes
Mar 20, 2026
Contributor
rhybrillou
left a comment
There was a problem hiding this comment.
I like the principle.
You'll need to fix other locations before merging (check the failed style job).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
https://www.jvt.me/posts/2025/02/12/go-omitzero-124/
User-facing documentation
Testing and quality
Automated testing
How I validated my change
CI