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

fix #416: Add skip-dirs-use-default #630

Merged
merged 1 commit into from Sep 10, 2019
Merged

fix #416: Add skip-dirs-use-default #630

merged 1 commit into from Sep 10, 2019

Conversation

Sean-Der
Copy link
Contributor

@Sean-Der Sean-Der commented Aug 5, 2019

Hopefully this is ok @jirfag!

This will make big difference for Pion. The more we crank up the linter the easier it gets to be a maintainer :) thanks again for golangci-lint, it has made a huge difference.

@Sean-Der
Copy link
Contributor Author

Sean-Der commented Aug 5, 2019

This is blocked/failing because of the SVG generation

This would be unblocked by merging #625

skipDirs = append(skipDirs, cfg.Run.SkipDirs...)
skipDirs := cfg.Run.SkipDirs
if cfg.Run.UseDefaultSkipDirs {
skipDirs = append([]string{}, packages.StdExcludeDirRegexps...)
Copy link
Member

Choose a reason for hiding this comment

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

Hey @Sean-Der, great to meet you :)

Does it mean that skip-dirs-use-default: true (which will be true by default) will replace the explicit skip-dirs setting?

	skipDirs := cfg.Run.SkipDirs // ["e2e", "foo/bar"]
	if cfg.Run.UseDefaultSkipDirs {
		skipDirs = append([]string{}, packages.StdExcludeDirRegexps...)
	}
	skipDirs // is now equal to packages.StdExcludeDirRegexps

Probably we want the skipDirs = append(skipDirs, packages.StdExcludeDirRegexps...) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ernado my name is Sean.

Oh nice catch, fixed!

@jirfag
Copy link
Member

jirfag commented Sep 10, 2019

Hi, thank you!
README.md needs to be regenerated, I will do it in master after merging

@jirfag jirfag merged commit f312a0f into golangci:master Sep 10, 2019
jirfag added a commit that referenced this pull request Sep 10, 2019
Also, output help for the new option in a more compact way.
@ldez ldez added this to the v1.18 milestone Mar 6, 2024
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.

None yet

4 participants