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(pre-commit): require_serial & pass_filenames #3713

Merged
merged 2 commits into from
Mar 23, 2023
Merged

Conversation

gnuletik
Copy link
Contributor

@gnuletik gnuletik commented Mar 21, 2023

In order to avoid the following error when running pre-commit

Error: parallel golangci-lint is running

We need to force golangci-lint to run in sequence.

Fixes #3715

@boring-cyborg
Copy link

boring-cyborg bot commented Mar 21, 2023

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2023

CLA assistant check
All committers have signed the CLA.

@ldez ldez changed the title feat(pre-commit): require_serial fix(pre-commit): require_serial Mar 22, 2023
@ldez ldez changed the title fix(pre-commit): require_serial fix(pre-commit): require_serial & pass_filenames Mar 22, 2023
@ldez ldez added area: pre-commit bug Something isn't working and removed area: docs labels Mar 23, 2023
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 1f4fed7 into golangci:master Mar 23, 2023
SeigeC pushed a commit to SeigeC/golangci-lint that referenced this pull request Apr 4, 2023
Co-authored-by: Fernandez Ludovic <ldez@users.noreply.github.com>
@macnibblet
Copy link

macnibblet commented Apr 24, 2023

@gnuletik @ldez To me after this has been merged the golangci-lint is now running against the entire code base instead of just the files changed when I commit. Are you experiencing the same issue?

@gnuletik
Copy link
Contributor Author

@macnibblet I just tried to:

  • Update a go file in my index (so golangci-lint is not skipped)
  • create a file with an error like bad.go
  • git add my updated go file in my index
  • commit

And yes, pre-commit is run on bad.go event if it should not.

I did not find a config to make it work properly.
If you set pass_filename: true, you may get:

ERRO [linters_context] typechecking error: named files must all be in one directory; have [some_package] and [another_package]

@macnibblet
Copy link

Hmm, I have never experienced that issue you are referring to. But can i set pass_filename as one of the args in my pre-commit config?

@gnuletik
Copy link
Contributor Author

Hmm, I have never experienced that issue you are referring to. But can i set pass_filename as one of the args in my pre-commit config?

Yes you can set pass_filename: true in your local config but you will get the above issue if you have multiple go packages.

@macnibblet
Copy link

@gnuletik so i tried setting the pass_filename: true but it works perfectly fine for us because all our source code is in a src folder

We have a mono repository that looks like this, so maybe we should update the documentation to explain the issue more clearly.

src/
  - authentication
  -- internal/
  -- cmd/

  - users
  -- internal/
  -- cmd/

@gnuletik
Copy link
Contributor Author

gnuletik commented May 6, 2023

@macnibblet even if all your source files are in a src directory, you may still have the issue because you have multiple go packages (e.g. users/cmd and authentication/cmd).

However, the reproduction may depends on your golangci-lint config and the files you include in your git commit.

See also #3200

@ldez ldez added this to the v1.52 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: pre-commit bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latest pre-commit hook is broken when used with --all-files or --files flag using multiple packages
4 participants