-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Using upstream goconst #1500
Using upstream goconst #1500
Conversation
jgautheron/goconst#13 must be merged and new version released before we proceed. |
@golangci/team, it seems that problem is more complicated here. Some time ago (our fork was based on 3-year old commit) new feature was introduced to Taking above into consideration we have 2 options, I think:
Second option sounds much more reasonable to me and I would file all the necessary PRs. Third option (that I consider a non-option) would be to keep maintaining our fork and deviate from the upstream. This is probably the worst possible idea. In case of no feedback I will proceed with option 2 (if @jgautheron is open to such a solution). |
Can we use default excludes to mitigate (1) or error messages are same in both cases? |
@ernado it should be possible, I will investigate. |
@ernado message is the same for all types of violation and I can't really tell one from another. |
f4abafb
to
0c4289c
Compare
I have file a PR against upstream |
4706e66
to
621a5f2
Compare
@ernado, a quick look would be appreciated :) |
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.
LGTM, thank you!
Thanks to jgautheron/goconst#11 we should be able to get rid of goconst fork. Closes #1495.