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

Update .pre-commit-hooks.yaml #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update .pre-commit-hooks.yaml #103

wants to merge 1 commit into from

Conversation

TylerYep
Copy link

pre-commit hooks should not always run on the current directory - formatters should only run on changed files.

@TylerYep
Update .pre-commit-hooks.yaml
567c55c 
pre-commit hooks should not run on the current directory - they should only run on changed files.
@matthewhughes934
Copy link

This would be very nice to have if possible, since it also means I could leverage pre-commit's exclude. I need this because I want to run this hook over a repo that has an intentional syntax error in a Go file https://gitlab.com/matthewhughes/go-cov/-/blob/81a617e1c8151f6f02f7118de80f1bffd123bfc3/testdata/syntax-error/syntax_error.go#L5 since I can't exclude that at the moment the hook just always fails

matthewhughes-uw added a commit to utilitywarehouse/manifest-checkers that referenced this pull request Feb 9, 2024
@matthewhughes-uw
(devena-mono) Add golines pre-commit hook
693afa0 
The motivation here is have to think less about code formatting when
writing code, e.g. " Should I break this long list of args up? Should I
add a newline between all of these args".

The `golangci-lint` team don't want to inlcude this there for reasons[1]
so run it instead via `pre-commit`.

This is using a fork which just has one extra commit[2] until [3] is
merged upstream

[1] golangci/golangci-lint#1914
[2] segmentio/golines@dedb5a0
[3] segmentio/golines#103
@echarrod
Copy link
Contributor

Think this is the right approach, sharing another MR which also wanted to change the same thing - #133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@echarrod echarrod echarrod approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TylerYep @matthewhughes934 @echarrod