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

Add more lint coverage #10049

Merged
merged 4 commits into from
Feb 7, 2022
Merged

Add more lint coverage #10049

merged 4 commits into from
Feb 7, 2022

Conversation

rosstimothy
Copy link
Contributor

@rosstimothy rosstimothy commented Jan 31, 2022

golangci-lint doesn't lint any subdirectories with their own go.mod (golangci/golangci-lint#828) which left a few directories unlinted. To get around this we can
run golangci-lint directly against those submodules.

This also provides fixes for new lint errors detected by running golangci-lint
in these directories.

@@ -102,8 +102,8 @@ ENV GOPATH="/go" \
RUN go install github.com/google/[email protected]

# Install meta-linter.
RUN (curl -L https://github.com/golangci/golangci-lint/releases/download/v1.38.0/golangci-lint-1.38.0-$(go env GOOS)-$(go env GOARCH).tar.gz | tar -xz && \
cp golangci-lint-1.38.0-$(go env GOOS)-$(go env GOARCH)/golangci-lint /bin/ && \
RUN (curl -L https://github.com/golangci/golangci-lint/releases/download/v1.44.0/golangci-lint-1.44.0-$(go env GOOS)-$(go env GOARCH).tar.gz | tar -xz && \
Copy link
Contributor

Choose a reason for hiding this comment

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

We will have to re-spin the buildbox, or update the linter script, in order for this update to make it onto CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I figured there would be more involved than this. But I was constantly getting errors that aren't picked up by CI and believe it is due to the older version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have PR #10179 that will remove some of the manual buildbox image management for GCB

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I think the comment here is out of date. If I had to guess, at some point we switched from go-metalinter to golangci-lint and left the old install comment.

@rosstimothy rosstimothy force-pushed the tross/better_coverage branch from e77024b to c0d4c9f Compare February 2, 2022 14:58
@rosstimothy rosstimothy marked this pull request as ready for review February 3, 2022 13:45
@github-actions github-actions bot requested a review from lxea February 3, 2022 13:46
@rosstimothy rosstimothy requested a review from tcsc February 3, 2022 13:46
golanglint-ci doesn't pick up subdirectories with their own go.mod
which left certain directories unlinted. To get around this we can
run golanglint-ci directly against those submodules.
@rosstimothy rosstimothy force-pushed the tross/better_coverage branch from 6cd6379 to 4ece427 Compare February 7, 2022 15:44
@rosstimothy rosstimothy merged commit 896261a into master Feb 7, 2022
@rosstimothy rosstimothy deleted the tross/better_coverage branch February 7, 2022 17:03
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