Re: [PATCH 2/2] contrib/git-candidate: Add README

On Wed, 2015-11-11 at 09:48 +0000, Richard Ipsum wrote:

quoted

quoted

+	(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.

quoted

"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.

quoted

quoted

+	(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.

quoted

quoted

+	(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.

quoted

quoted

+	(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.