Skip to content

Conversation

@tarasmadan
Copy link
Collaborator

It contributes to #4317 unblocking.

dvyukov
dvyukov previously approved these changes Nov 8, 2023
@tarasmadan
Copy link
Collaborator Author

@a-nogikh ptal

Copy link
Collaborator

@a-nogikh a-nogikh left a comment

Choose a reason for hiding this comment

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

Are there global ignore strings list? Maybe [syzbot] could belong there. I think tests are easier to follow if it's a string rather than if it's some referenced const.

Also, here we patch it when it's used more explicitly, but there are also cases where it's a part of a title like "[syzbot] fix bug title" from email_test.go, and it was just not caught by the linter.

@tarasmadan
Copy link
Collaborator Author

tarasmadan commented Nov 9, 2023

No. The only alternative I see is to use "// nolint: goconst" in every line with "[syzbot]".
If it looks better for you please let me know.

@a-nogikh
Copy link
Collaborator

a-nogikh commented Nov 9, 2023

Maybe there are at least per-file annotations? There must be.

@tarasmadan
Copy link
Collaborator Author

tarasmadan commented Nov 10, 2023

https://golangci-lint.run/usage/linters/#goconst is about configuration.
The required flexibility is absent.

jgautheron/goconst#22 to increase this 3 days long discussion value.
Once landed, golangci-lint change will be needed to support the new flag.

@tarasmadan
Copy link
Collaborator Author

Added to goconst tool.
Next actions are to:

  1. Wait for dependabot update on https://github.com/golangci/golangci-lint.
  2. Add required option support or just request the feature there.

@codecov
Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #4323 (032a471) into master (fc59b78) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@tarasmadan
Copy link
Collaborator Author

golangci/golangci-lint#4200 landed the feature.
I think we'll have it the next golangci-lint release.

@tarasmadan
Copy link
Collaborator Author

#4374 landed it

@tarasmadan tarasmadan closed this Dec 6, 2023
@tarasmadan tarasmadan deleted the const_syzbot_prefix branch February 14, 2024 14:35
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