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

x/review: doc: git codereview mail error msg should mention git codereview hooks #26773

Closed
ijt opened this issue Aug 2, 2018 · 3 comments
Closed

x/review: doc: git codereview mail error msg should mention git codereview hooks #26773

ijt opened this issue Aug 2, 2018 · 3 comments
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@ijt
Copy link
Contributor

ijt commented Aug 2, 2018

I was getting the following error when attempting to mail my change for review:

ijt:~/github/go/src$ git codereview mail                                                                          
remote: Processing changes: refs: 1, done                        
remote: ERROR: [c442702] missing Change-Id in commit message footer
remote:                                                                                                                                                       
remote: Hint: to automatically insert a Change-Id, install the hook:                                                            
remote: f=`git rev-parse --git-dir`/hooks/commit-msg ; curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f
remote: and then amend the commit:                            
remote:   git commit --amend                                      
remote:         
To https://go.googlesource.com/go
 ! [remote rejected]       HEAD -> refs/for/master ([c442702] missing Change-Id in commit message footer)
error: failed to push some refs to 'https://go.googlesource.com/go'
(running: git push -q origin HEAD:refs/for/master)

Running git codereview hooks fixes the problem but the error message doesn't mention this fix and neither does the Contribution guide.
@ijt ijt changed the title doc: Contribution guidelines should mention git codereview hooks doc: contribution guide should mention git codereview hooks Aug 2, 2018
@ALTree
Copy link
Member

ALTree commented Aug 2, 2018 •
edited

I don't think this should go in the contribution guide. I've never seen that error before. My guess is that you did something unusual when setting up the repo for contributing or when you created your new change.

I think you understand that if we were to mention every possible error message you may encounter, and explain how to solve it, the guide would be like 10x larger. Considering how large it already is, I don't think that would be ideal. Describing commonly encountered errors may be worth it, but this is not one of them.

If you don't want your contribution guide to be huge and overwhelming, the best thing to do is to describe a "happy path" (as a sequence of commands), that new contributors are supposed to follow strictly. Describing every possible variant, every possible error you may encounter in unusual situations, and how to solve it would make the guide too verbose.

The error message, OTOH, could be updated with better instructions, if possible.

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 2, 2018
@ijt ijt changed the title doc: contribution guide should mention git codereview hooks doc: git codereview mail error msg should mention git codereview hooks Aug 2, 2018
@ianlancetaylor ianlancetaylor added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 3, 2018
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Aug 3, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 3, 2018
@ianlancetaylor
Copy link
Contributor

I also have never seen that. It would help if you could show us a set of commands, starting from scratch, that demonstrates the problem.

@ianlancetaylor ianlancetaylor changed the title doc: git codereview mail error msg should mention git codereview hooks x/review: doc: git codereview mail error msg should mention git codereview hooks Aug 3, 2018
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Sep 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@ijt @ianlancetaylor @ALTree @gopherbot