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

go-lint pre-commit only applies to staged changes #382

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

YrrepNoj
Copy link
Contributor

@YrrepNoj YrrepNoj commented Mar 11, 2022

The current pre-commit hook applies linting to all of the *.go files in the repo. I think this was necessary back when we had multiple go.mod and go.sum files in the repo. Either way, it doesn't seem to be necessary anymore. Now the pre-commit config will only run the linting against files that are staged for commit. This also uses the same repo we use to run our other go pre-commit tasks.

NOTE: It will run against the entire file, not just the parts that have been updated.

This screenshot shows the pre-commit linting failing on only the file staged and not on the other linting issues we currently have:
Screen Shot 2022-03-11 at 12 30 16 AM

@YrrepNoj YrrepNoj requested a review from jeff-mccoy March 11, 2022 05:23
@YrrepNoj YrrepNoj self-assigned this Mar 11, 2022
@YrrepNoj YrrepNoj requested a review from RothAndrew March 11, 2022 16:44
.pre-commit-config.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@RothAndrew RothAndrew left a comment

Choose a reason for hiding this comment

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

Change

  - repo: https://github.com/dnephin/pre-commit-golang
    rev: v0.4.0
    hooks:
      - id: go-fmt
  - repo: https://github.com/dnephin/pre-commit-golang
    rev: v0.4.0
    hooks:
      - id: golangci-lint

to

  - repo: https://github.com/dnephin/pre-commit-golang
    rev: v0.4.0
    hooks:
      - id: go-fmt
      - id: golangci-lint

RothAndrew
RothAndrew previously approved these changes Mar 14, 2022
@YrrepNoj
Copy link
Contributor Author

@RothAndrew I had to re-push to fix a merge conflict, when you have a moment could you re-review? Thanks.

@YrrepNoj YrrepNoj merged commit 7f2379d into master Mar 14, 2022
@YrrepNoj YrrepNoj deleted the update-lint-pre-commit-hook branch March 14, 2022 17:46
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
* lint pre-commit only applies to staged changes
* using remote golangci/golangci-lint instead of a local hook
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