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

Pass patch file to golangci-lint, align with CI #2365

Merged
merged 1 commit into from
Jan 19, 2022
Merged

Conversation

lucacome
Copy link
Member

This should align local runs of golangci-lint with CI runs.

@lucacome lucacome requested a review from a team January 19, 2022 03:38
@lucacome lucacome self-assigned this Jan 19, 2022
@lucacome lucacome requested review from pleshakov and ciarams87 and removed request for a team January 19, 2022 03:38
@github-actions github-actions bot added the chore Pull requests for routine tasks label Jan 19, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2022

Codecov Report

Merging #2365 (22a81bc) into master (7a702f8) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2365      +/-   ##
==========================================
+ Coverage   53.66%   53.69%   +0.02%     
==========================================
  Files          48       48              
  Lines       14201    14201              
==========================================
+ Hits         7621     7625       +4     
+ Misses       6340     6338       -2     
+ Partials      240      238       -2     
Impacted Files Coverage Δ
internal/k8s/configuration.go 95.86% <0.00%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a702f8...22a81bc. Read the comment docs.

@@ -29,7 +29,8 @@ all: test lint verify-codegen update-crds debian-image

.PHONY: lint
lint: ## Run linter
docker run --pull always --rm -v $(shell pwd):/kubernetes-ingress -w /kubernetes-ingress -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:latest golangci-lint --color always run -v
@git fetch
docker run --pull always --rm -v $(shell pwd):/kubernetes-ingress -w /kubernetes-ingress -v $(shell go env GOCACHE):/cache/go -e GOCACHE=/cache/go -e GOLANGCI_LINT_CACHE=/cache/go -v $(shell go env GOPATH)/pkg:/go/pkg golangci/golangci-lint:latest git diff -p origin/master > /tmp/diff.patch && golangci-lint --color always run -v --new-from-patch=/tmp/diff.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we make the diff against the parent branch rather than the master?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think usually the patch file is generated against the main branch? That 99% of the time is also the parent I guess 😅
I'm not sure how you would easily get the parent, I found this on stackoverflow https://stackoverflow.com/questions/3161204/how-to-find-the-nearest-parent-of-a-git-branch but the proposed solution doesn't work for me, and the second one looks like a lot of "magic" 😄

I was trying to reproduce the behavior of golangci-lint, which gets the patch file for a PR from GitHub API, like this https://github.com/nginxinc/kubernetes-ingress/pull/2365.patch

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@lucacome lucacome enabled auto-merge (squash) January 19, 2022 20:48
@lucacome lucacome merged commit 7366e07 into master Jan 19, 2022
@lucacome lucacome deleted the chore/fix-linter branch January 19, 2022 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants