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

[simo]

On Mon, 2012-10-22 at 21:17 -0400, simo wrote:
> 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.

Ah one last thing, I have not hooked up patchwork to our master git repo
with a hook, I am investigating if I can instead use a git pull driven
by a cronjob on my machine instead to avoid needing hooks elsewhere. It
makes it easier as not all repositories let you add hook easily.

If you know how to hook stuff via pull let me know.

Simo.

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