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

x/review/git-codereview: mail must update the reviewer list even if there are no staged changes #11192

Open
rakyll opened this issue Jun 12, 2015 · 2 comments
Milestone

Comments

@rakyll
Copy link
Contributor

rakyll commented Jun 12, 2015

mail rejects to update the reviewer list if there are no staged changes. This behaviour is breaking a common use-case where the contributor creates a CL with no reviewers, review his/her changes on Gerrit and add reviewers. Allow mail subcommand to update the reviewers list even if there are no changes.

@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 12, 2015
@adg
Copy link
Contributor

adg commented Jun 14, 2015

Is it this behavior that you're referring to?

$ git mail 
remote: Processing changes: new: 1, done    
remote: 
remote: New Changes:
remote:   https://go-review.googlesource.com/11095 test: test commit, please ignore
remote: 
$ git mail  -r bradfitz
remote: Processing changes: refs: 1, done    
To https://go.googlesource.com/build
 ! [remote rejected] HEAD -> refs/for/master%r=bradfitz@golang.org (no new changes)
error: failed to push some refs to 'https://go.googlesource.com/build'
(running: git push -q origin HEAD:refs/for/master%r=bradfitz@golang.org)
git-codereview: exit status 1

This is the Gerrit server rejecting push, not the git-codereview tool.

I agree this workflow should work. I can see two ways of making this happen:

  1. Fix it on Gerrit's side (paging @spearce, or I guess we could file an issue),
  2. Have the git-codereview tool do a git diff HEAD $BRANCH.mailed and—if there are no changes—use the Gerrit API to adjust the reviewers list instead of doing a git push.

I'd prefer to do the former rather than the latter. If @spearce indicates that this can happen on the Gerrit side, then I'll close this issue. Otherwise we should think about working around it on the git-codereview side.

@rakyll
Copy link
Contributor Author

rakyll commented Jun 14, 2015

Yes, it's the behaviour above I was referring to. I assume the latter solution is easy to implement and it must relatively harder to get this done at the Gerrit side not to break the current behavioral compatibility.

@mikioh mikioh changed the title x/review: mail must update the reviewer list even if there are no staged changes x/review/git-codereview: mail must update the reviewer list even if there are no staged changes Aug 17, 2015
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

No branches or pull requests

3 participants