Review process (was: Re: [PATCH] libcli/dns: Time out requests after a while)
jra at samba.org
Tue Oct 16 04:03:11 MDT 2012
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.
> Ah, I take that's the famous ninja review? ;)
They're always the hardest :-).
More information about the samba-technical