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.

Sign up for GitHub

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

Jump to bottom

Revert exclude package-comments in golangci-lint #1011

Merged
merged 2 commits into from
Dec 22, 2022
Merged

Revert exclude package-comments in golangci-lint #1011

merged 2 commits into from
Dec 22, 2022

Conversation

roger2hk
Copy link
Contributor

@roger2hk roger2hk commented Dec 2, 2022

Checklist

@roger2hk roger2hk requested a review from AlCutter December 2, 2022 09:28
@roger2hk roger2hk marked this pull request as ready for review December 2, 2022 09:28
@roger2hk roger2hk requested a review from a team as a code owner December 2, 2022 09:28
@AlCutter
Copy link
Member

AlCutter commented Dec 9, 2022

Do we still want to merge this now that we have doc comments for all packages from #1012 ?
I'm thinking it might be nice to enforce that we have them for any new packages going forward via the linter check, wdyt?

@roger2hk roger2hk force-pushed the exclude-package-comments branch 2 times, most recently from 816c49b to 13c9553 Compare December 15, 2022 14:58
@roger2hk roger2hk closed this Dec 15, 2022
@roger2hk roger2hk force-pushed the exclude-package-comments branch from 13c9553 to ee2cd5d Compare December 15, 2022 15:00
@roger2hk roger2hk reopened this Dec 15, 2022
@roger2hk roger2hk changed the title Exclude package-comments in golangci-lint Revert exclude package-comments in golangci-lint Dec 15, 2022
@roger2hk
Copy link
Contributor Author

Do we still want to merge this now that we have doc comments for all packages from #1012 ? I'm thinking it might be nice to enforce that we have them for any new packages going forward via the linter check, wdyt?

This PR reverts the "exclude package-comments in golangci-lint". The presubmit is expected to fail because there are some packages missing the comment.

@roger2hk roger2hk force-pushed the exclude-package-comments branch from 8cf7e9f to 917696c Compare December 16, 2022 16:29
@AlCutter
Copy link
Member

Missed one: scanner/scanlog/scanlog.go:15:1: package-comments: should have a package comment (revive) :/

@roger2hk
Copy link
Contributor Author

Missed one: scanner/scanlog/scanlog.go:15:1: package-comments: should have a package comment (revive) :/

Actually there are 2 remaining issues.

➜  certificate-transparency-go git:(exclude-package-comments) golangci-lint run --print-issued-lines=false --max-same-issues=0 | wc -l
2

Another one comes from the preloader.

preload/preloader/preloader.go:15:1: package-comments: should have a package comment (revive)

@roger2hk roger2hk merged commit ce6530b into google:master Dec 22, 2022
@roger2hk roger2hk deleted the exclude-package-comments branch December 22, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlCutter AlCutter AlCutter approved these changes

Assignees

@AlCutter AlCutter

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@roger2hk @AlCutter