On Wed, 2015-11-11 at 09:48 +0000, Richard Ipsum wrote: > > > > > + (master)$ git candidate submit origin archiverepo > > > + Review added successfully > > > > Is the contributor automatically (optionally) emailed on this? If not, > > consider this a feature request for this. > > There's no server integration of any kind at the moment, > this is clearly something we will want to add.
I don't think this needs server integration. It could just work like git send-email and send the email from the local machine. > > "git candidate diff" might be nice too to show the diff between v1 and > > v2. You might even have "git candidate commit-diff" (or some better > > name) so you can see which commit has changed in a changeset containing > > multiple commits. > > Yes, we definitely want that. I think "git candidate diff" to diff > between revisions would be sufficient, and it could take a list of files > to diff as an arg? That's a good start, but often I want to review per-patch, so it would be nice (if complicated) to track the evolution of a patchset. > > > + (master)$ git candidate review origin/archiverepo --vote +2 > > > + -m "Looks good, merging. Thanks for your efforts" > > > + Review added successfully > > > > Is that +2 "+1 because I like it, +1 because I previously -1'd it?" If > > so, it might be nice to have --replace-vote so you don't have to track, > > "wait, I did -1, then +1, then -1 again..." > > Votes are per-review, perhaps they should simply be per-revision? > Then --vote sets the vote for the revision and there's no need for > a --replace-vote option? > This would use user.name and user.email as identification. I like votes being per-revision. > > > + (master)$ git candidate submit origin archiverepo > > > + Candidate was submitted successfully. > > > > I don't understand what the verb "submit" means here. Is it "mark this > > as accepted"? If so, "accept" might be a better word. > > I'm tempted to change this to 'push', 'submit' comes from gerrit. SGTM. > > > + (master)$ git merge candidates/origin/archiverepo > > > > I would like "git candidate merge" to do a submit+merge the way that > > pull does a fetch+merge. It seems like the common case. Also, if it > > turns out at this point that there's a merge conflict, I might want to > > back out the acceptance. > > There is currently no git-candidate-merge, I removed this recently > because I decided that you can merge candidates with git-merge > and that this is more flexible. Often a candidate will be rebased > before it is merged, it would be nice to avoid having to create > a merge command that needs to handle all the different cases for > merging a candidate. I like the convenience, but it could always be added later. One more random note: it might be nice to have a Documentation/technical article describing review storage. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
