Skip to content

build/ci: update golangci-linter with --new-from-rev #32262

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

Closed
wants to merge 3 commits into from

Conversation

Muzry
Copy link

@Muzry Muzry commented Jul 23, 2025

By using --new-from-rev=origin/master with golangci-lint, it only lint the files that have changed compared to origin/master, reducing the linting workload.

@Muzry Muzry changed the title update golangci-linter run with --new-from-rev=origin/main build/ci: update golangci-linter run with --new-from-rev=origin/main Jul 23, 2025
@Muzry Muzry changed the title build/ci: update golangci-linter run with --new-from-rev=origin/main build/ci: update golangci-linter with --new-from-rev Jul 23, 2025
@MariusVanDerWijden
Copy link
Member

level=warning msg="[runner] Can't process result by diff processor: can't prepare diff by revgrep: could not read git repo: error executing "git diff --color=never --no-ext-diff --default-prefix --relative origin/master --": exit status 128: fatal: bad revision 'origin/master'\n"

As I expected, this does not work in our CI

@Muzry Muzry force-pushed the update_go_ci_linter branch from b62719e to 8b5a222 Compare July 23, 2025 09:56
@Muzry
Copy link
Author

Muzry commented Jul 23, 2025

level=warning msg="[runner] Can't process result by diff processor: can't prepare diff by revgrep: could not read git repo: error executing "git diff --color=never --no-ext-diff --default-prefix --relative origin/master --": exit status 128: fatal: bad revision 'origin/master'\n"

As I expected, this does not work in our CI

@MariusVanDerWijden Thanks for your reply and for helping fix the branch issue. I have updated the configs and fixed the problem. Please review again.

@MariusVanDerWijden
Copy link
Member

Its still not faster... The lint was 5 min before and is still 5 min now. I don't think it makes sense to add this, sorry. We always want the linter to run on all the files in the repository, since renamings can affect non-changed files and this would miss that

@Muzry
Copy link
Author

Muzry commented Jul 27, 2025

Its still not faster... The lint was 5 min before and is still 5 min now. I don't think it makes sense to add this, sorry. We always want the linter to run on all the files in the repository, since renamings can affect non-changed files and this would miss that

Appreciate the feedback — I’ll address the other issues and close this PR for now.

@Muzry Muzry closed this Jul 27, 2025
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.

2 participants