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

Jump to bottom

misc/git/pre-commit: complains about unformatted files that are not staged #23136

Closed
ErinCall opened this issue Dec 14, 2017 · 6 comments
Closed

misc/git/pre-commit: complains about unformatted files that are not staged #23136

ErinCall opened this issue Dec 14, 2017 · 6 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@ErinCall
Copy link

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

What did you do?

cd some/project/using/misc/git/pre-commit
$EDITOR some/file.go
# make some changes with *valid* formatting
git add some/file.go
# make some more changes with *invalid* formatting
# (but don't add them)
git commit

(In short: have some staged changes with valid formatting and some unstaged changes with invalid formatting)

What did you expect to see?

Everything's hunky-dunky; the changes I'm committing have valid formatting

What did you see instead?

Go files must be formatted with gofmt. Please run: ...
ErinCall added a commit to ErinCall/go that referenced this issue Dec 14, 2017
@ErinCall
pre-commit: Only run go fmt on staged changes [golang#23136]
1fca4db 
If there is a formatting problem but it isn't staged, then it isn't
relevant to the commit in progress. It might be a blocker eventually,
but it isn't yet.
ErinCall pushed a commit to ErinCall/go that referenced this issue Dec 14, 2017
git/pre-commit: remove tempdir when finished [golang#23136]
5bfb15b
ErinCall added a commit to ErinCall/go that referenced this issue Dec 14, 2017
@ErinCall
git/pre-commit: remove tempdir when finished [golang#23136]
f85f35a
ErinCall added a commit to ErinCall/go that referenced this issue Dec 14, 2017
@ErinCall
pre-commit: Only run go fmt on staged changes [golang#23136]
932dfe4 
If there is a formatting problem but it isn't staged, then it isn't
relevant to the commit in progress. It might be a blocker eventually,
but it isn't yet.

Change-Id: I5120407a4bd4cc047672aa10a74a530613e8c0bb
ErinCall added a commit to ErinCall/go that referenced this issue Dec 14, 2017
@ErinCall
git/pre-commit: remove tempdir when finished [golang#23136]
0d15ce7 
Change-Id: I56d524b98fc101199ae8a44ba8e3d5162c6f177e
@ErinCall
Copy link
Author

I've given up on trying to figure out the preposterous prerequisites for using google's contribution system but ErinCall:precommit-staged-changes-only has a working fix and you can have it if you want

@odeke-em
Copy link
Member

Hello there @ErinCall, thank you for filing this issue.

Please don't lose hope about contributing, it gets a whole lot better once we get over such humps :)
That precommit hook exists to ensure that we don't have un-gofmt'd files then accidentally submit a patchset, but I understand how frustrating that could be.
Given that you've already git add-ed that file, makes the file fair game for precommit scrutiny to help one not lose their changes.

With that said though, I'll ping some git experts for some ideas @rsc @bradfitz @ianlancetaylor.

@ianlancetaylor
Copy link
Contributor

The Go project does accept GitHub pull requests now, if you feel like trying again. Thanks.

@ianlancetaylor ianlancetaylor changed the title misc/git/pre-commit may complain about unstaged formatting problems misc/git/pre-commit: complains about unformatted files that are not staged Mar 29, 2018
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@ErinCall ErinCall mentioned this issue Apr 3, 2018
Closed
@gopherbot
Copy link

Change https://golang.org/cl/104575 mentions this issue: git/pre-commit: check formatting for staged changes only

@agnivade
Copy link
Contributor

I dont think this file is even used anymore. My .git/hooks/pre-commit has exec git-codereview hook-invoke %s "$@". Which means this is being handled by the code-review tool.

And it looks like this behavior is controlled by a flag which needs to be set for it to check unstaged changes also. https://github.com/golang/review/blob/master/git-codereview/gofmt.go#L53. If that is satisfactory behavior, I think this file can be deleted and the issue closed.

@agnivade
Copy link
Contributor

agnivade commented Mar 4, 2019

This is done with 0c7cdb4. Closing.

@agnivade agnivade closed this as completed Mar 4, 2019
@golang golang locked and limited conversation to collaborators Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@ErinCall @agnivade @ianlancetaylor @odeke-em @gopherbot