-
Notifications
You must be signed in to change notification settings - Fork 594
chore(ci): upgrade to golangci-lint 2.6.1 #4973
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
Conversation
julio-lopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇Thanks for doing this.
And thanks for splitting it into self contained, easily reviewable commits. It simplifies and speeds up the review process.
The omitempty changes (in the last commit) probably need to be reverted for now. Even though omitempty is ineffective on structs, there is a test that checks for the presence of these tags, it really checks for an equivalence between the "policy definition source" and the structs containing the actual values.
| if sleepInterval < minWaitInterval { | ||
| sleepInterval = minWaitInterval | ||
| } | ||
| sleepInterval := max(min(maxWaitTime/10, maxWaitInterval), minWaitInterval) //nolint:mnd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be split for clarity?
| sleepInterval := max(min(maxWaitTime/10, maxWaitInterval), minWaitInterval) //nolint:mnd | |
| sleepInterval := min(maxWaitTime/10, maxWaitInterval) //nolint:mnd | |
| sleepInterval = max(sleepInterval, minWaitInterval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm accustomed to seeing min/max composed together to "clamp" a value, but happy to split if you prefer.
This reverts commit a6d7971.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4973 +/- ##
==========================================
+ Coverage 75.86% 78.02% +2.15%
==========================================
Files 470 548 +78
Lines 37301 31412 -5889
==========================================
- Hits 28299 24508 -3791
+ Misses 7071 4853 -2218
- Partials 1931 2051 +120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The `omitempty` JSON tag is ineffective on fields of struct type, among others. The fields with tags changed from `omitempty` to `omitzero` can be classified into 2 categories: - fields for which the intent was to avoid serializing the value when it was zero or empty (such as structs with the zero value for the struct); - policy-definition fields that have the `omitempty` to match the corresponding policy fields. The fields are changed such that - the fields are not serialized when they have the zero (or empty) value; and - consistency is maintained between policy fields and the corresponding policy definition fields. --- In the context of policy definitions: **Fields of type `struct`**: In some cases, the fields in the policy definition and values are of type struct, such as is the case for the `policy.Policy` and `policy.Definition` structs. In these cases, the `omitzero` JSON tag avoids marshaling empty fields, making the serialized representation more compact and less noisy, while preserving the same behavior and thus semantic when unmarshaling omitted fields. **Fields of pointer types**: The `omitempty` and `omitzero` have _practically_ the same effect on fields of pointer types: - the field is omitted when it is null - the field is included when it is not null, even if the value that it points to is "the zero value" for the non-pointer type. Note: when the pointer type defines an `IsZero()` member function, then that field would also be omitted during marshaling. There are no defined `IsZero()` function for these pointers, so the semantics are preserved in this case. The `omitzero` JSON tag in the fields definition structs, such as the `ActionPolicyDefinition struct`, does not change the semantics, it simply makes the marshaled representation more compact. **Fields of type slice**: The behavior for `omitempty` and `omitzero` differs for slices, and maps as well. The struct fields of slice type, such as `[]string`, are left with the `omitempty` tag to be able to tell the difference between a nil slice and a non-nil, zero-length slice. Even though, currently most code paths do not explicitly differentiate between a nil slice and an empty slice, the `omitempty` tag is left unmodified out of abundance of caution. --- Ref: - #4973 - #4907
Keeping up the momentum of #4931. A new minor version of golangci-lint was released after that PR was opened. One commit per linter as before.