Skip to content
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

Documentation on contributing #1647

Merged
merged 1 commit into from
Feb 8, 2021
Merged

Documentation on contributing #1647

merged 1 commit into from
Feb 8, 2021

Conversation

butuzov
Copy link
Member

@butuzov butuzov commented Jan 12, 2021

This PR fix the reference to govet in contributing docs as linter that has one option. Not critical, just cleaning.

@ldez
Copy link
Member

ldez commented Jan 12, 2021

Hello,

could you explain a bit more the change?

@ldez ldez added the feedback required Requires additional feedback label Jan 12, 2021
SVilgelm
SVilgelm previously approved these changes Jan 12, 2021
@SVilgelm SVilgelm dismissed their stale review January 12, 2021 20:35

sorry, didn't see that tit was changed from covet to unparam

@SVilgelm
Copy link
Member

@butuzov Yeah, could you please explain why did you change the example from govet to unparam? I really don't see a reason to make this change

@butuzov
Copy link
Member Author

butuzov commented Jan 13, 2021

Hi Guys.

I had run into an issue yesterday, that govet hasn't reported for shadoowing variables/packages. That snowballed into some investigation and this PR one of the outcomes.


This is minor change to the website docs and can be left in place, but I guess we will skip the initial frustration of the person reading at the website "For example, govet has only 1 option: check-shadowing.", then looking to the GovetSettings struct and finding that there are more. I have looked for the next linter that has only one setting (that can be passed via CLI flag).

What I have missed is that there is another option for unparam (one not passed to CLI. So here are the options we have:

  1. Replace govet with other linter with only one setting (next in the list is nakedret)
  2. Close this issue and left govet in tutorial as "linter with only one option"/s

@butuzov
Copy link
Member Author

butuzov commented Jan 13, 2021

Again, this quite a minor thing, and not critical.

`govet` is replaced by `nakedret` as example of the function that has
only one option in settings.
@ldez ldez removed the feedback required Requires additional feedback label Feb 8, 2021
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 65369cb into golangci:master Feb 8, 2021
ashanbrown pushed a commit to ashanbrown/golangci-lint that referenced this pull request Feb 20, 2021
@butuzov butuzov deleted the govet-docs branch February 21, 2021 04:06
@ldez ldez added this to the v1.37 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants