review tools (was Re: Code review required for commits - formal Team vote.)

[simo]

On Mon, 2012-10-22 at 21:13 -0400, simo wrote:
> On Mon, 2012-10-22 at 20:22 -0400, Scott Lovenberg wrote:
> > On Mon, Oct 22, 2012 at 6:26 PM, simo <idra at samba.org> wrote:
> > > So there is one gotchs with patchwork. If you send multiple patches in
> > > the same mail message only the last one will be picked up.
> > >
> > > Using git send-mail will send one patch per mail and make the whole
> > > process of tracking them easier
> > > I suggest people try to use it or anyway send a separate mail per patch.
> > >
> > > Simo.
> > 
> > If I may offer just a bit of advice as someone who fought with git
> > send-email and friends (because, what's coding without fighting with a
> > tool chain :) )...
> > 
> > * I've found it's easier to first use git format-patch and then
> > include all of the patch files from that in git send-email.
> > * I always annotate when I do a git send-email.  It's your last chance
> > to double check your commit message.
> > * Thunderbird and GMail will chew on your patchset (usually inserting
> > and removing whitespace - perfect if your patch is Python).
> > * I also had to change my Gmail settings to force GMail to use UTF-8
> > for outgoing messages to avoid weirdness.
> > * Discussion about a patchset is easier when each patch is in its own
> > email, IMHO.
> > 
> > HTH someone out there.
> 
> Thanks Scott,
> I'll attach here a bunch of advice I sent to the freeipa-devel mailing
> list for which project I also set up a patchwork project on the same
> server:
> 
> ---
> In case you wonder how to use git send-mail here are a couple of things
> I do to make it easier.
> 
> 1. I cloned my public review repo on the machine I use for email so I
> can push directly from there even when the patches are built on my
> development machines.
> 
> 2. set the [sendemail] option for the outgoing smtp server in
> ~/.gitconfig
> 
> 3. I create 2 aliases that make the process just s simple 2 commands:
> 
> alias prep-freeipa='rm -fr $HOME/git-send-mail && git format-patch -M -C --patience --full-index -n --cover-letter -o $HOME/git-send-mail'
> alias send-freeipa='git send-email --no-chain-reply-to --to "freeipa-devel.redhat com" --suppress-cc=all $HOME/git-send-mail/*patch'

Ah somehow my cut&paste chew the '@' sign and of course you'd want to
send to samba-technical, so thi is how a send-samba alias would look:

alias send-samba-technical='git send-email --no-chain-reply-to --to "samba-technical at samba.org" --suppress-cc=all $HOME/git-send-mail/*patch'

Simo.

> This first alias is called like this:
> $ prep-freeipa -3
> 
> It will create a bundle for the lst 3 patches in the tree and dump
> patches as well as a standard cover letter in a directory called
> $HOME/git-send-mail
> 
> I then vim $HOME/git-send-mail/0000-cover-letter.patch, where you need
> to add a subject and fill in the body of the presentation email
> 
> Then call simply send-freeipa without any option.
> It will ask a couple of questions to which you can normally just hit
> return (defaults are usually ok).
> ---
> 
> HTH,
> Simo.
> 


-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>