Review process (was: Re: [PATCH] libcli/dns: Time out requests after a while)

[Jeremy Allison]

On Tue, Oct 16, 2012 at 11:53:15AM +0200, Kai Blin wrote:
> 
> I only do my famous mind-reading trick on Thursdays, so I couldn't tell
> somebody was already tracking the patch.

:-). I track *everything*. I just don't always *do* anything
about it :-).

> Note that I did CC Matthieu on the first patch and Volker on the second.

You did indeed. I'm sorry for not noticing that.

> I'm sending patches straight from my git tree using "git-format-patch |
> git send-email", so the email text is actually the commit message.
> Apparently that's not the correct way to do this.

Actually I rather think it is. I've been writing separate
messages but either should be acceptable IMHO.

> So how does the protocol work? I'm the main person working on the
> libcli/dns code. In fact, I'm the maintainer of that code. How do I
> pick the reviewer?

Experience :-). For this patch (a tevent fix) vl, metze
or Michael. But I will do in a pinch :-).

> Could someone walk me through the whole process here?
> 
> 1. I have written a patch that I want to get into the tree.
> 2. I put the patch somewhere for a review. Where?

samba-technical and also explicitly to the reviewers you
think should be involved.

> 3. Someone reviews the patch. Who?

A Team member (or one of the explicit reviewers).

> 4. a) Reviewer pushes patch to autobuild
>    b) Reviewer doesn't like the patch, fix patch and goto 1.
> 5, a) Patch is in master, life is good.
>    b) autobuild rejects patch.
>       i) autobuild test run was flakey, autobuild had a hangover or
>          whatever, is that a goto 4a) or a goto 1.?

That's a 4a) I think. Autobuild failed for the reviewer, then that's
their responsibility to re-push (IMHO).

>      ii) autobuild rejected the patch because it broke a test, fix and
>          goto 1.
> 
> As you can see I'm a bit fuzzy on the details for steps 2, 3, and 5bi,
> I'd be happy if someone could shed some light on this

Tried to inline above.

> > As soon as you raised this on the list (even without asking
> > a perticular person) it got pushed. I don't think that's too
> > bad IMHO.
> 
> Sure, that's what I was expecting. Otherwise I'd have just pushed it to
> autobuild. I probably should have written up this email right away,
> instead of the previous one. Sorry about that.

No problem.

> Ah, I take that's the famous ninja review? ;)

They're always the hardest :-).

Jeremy.