Skip to content

fix: ignore fields of json:"-" #39

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

Merged
merged 2 commits into from
Feb 24, 2025
Merged

Conversation

tsuzu
Copy link
Contributor

@tsuzu tsuzu commented Feb 18, 2025

https://pkg.go.dev/encoding/json#Marshal

  • Ignore json:"-" fields in extractjsontags helper and commentstart analyzer
// Field is ignored by this package.
Field int `json:"-"`
  • Don't ignore json:"-,"
// Field appears in JSON as key "-".
Field int `json:"-,"`

@JoelSpeed
Copy link
Contributor

Please can you add a test case for the "-," case so that we make sure that's still working?

@tsuzu
Copy link
Contributor Author

tsuzu commented Feb 20, 2025

Please can you add a test case for the "-," case so that we make sure that's still working?

Sure, added!

Comment on lines +84 to 85
//nolint:cyclop
func extractTagInfo(tag *ast.BasicLit) FieldTagInfo {
Copy link
Contributor Author

@tsuzu tsuzu Feb 20, 2025

Choose a reason for hiding this comment

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

pkg/analysis/helpers/extractjsontags/analyzer.go:84:1: calculated cyclomatic complexity for function extractTagInfo is 12, max is 10 (cyclop)
func extractTagInfo(tag *ast.BasicLit) FieldTagInfo {

I've got a linter error for extractTagInfo. Can I add nolint for this func? I don't have a good idea to split this func.

JoelSpeed
JoelSpeed previously approved these changes Feb 21, 2025
@JoelSpeed
Copy link
Contributor

Something appears to have gone wrong with the linting here 🤔 Do you have time to investigate? I'm on PTO today

@tsuzu
Copy link
Contributor Author

tsuzu commented Feb 21, 2025

Enjoy your time off!

The error seems to be due to Go 1.24.0 release. And the support of Go 1.24.x for golangci-lint is still WIP: golangci/golangci-lint#5225
I've pinned golangci-lint job to Go 1.23, as well as the build job.

@ldez
Copy link

ldez commented Feb 23, 2025

the support of Go 1.24.x for golangci-lint is still WIP

The support of Go 1.24 is not WIP since v1.64.0, I just left the issue open to catch feedback.
There is a message in bold inside the issue description about the availability of the support of go1.24, but this seems unclear, so I just closed the issue now.

@JoelSpeed JoelSpeed merged commit 3114794 into kubernetes-sigs:main Feb 24, 2025
2 checks passed
@tsuzu
Copy link
Contributor Author

tsuzu commented Feb 25, 2025

Ahh, sorry! I missed it. Thank you so much for information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants