Reviewer-push (was: Re: [PATCH 00/16] Get rid of obsolete system getpass().)

[Jelmer Vernooij]

On Fri, 2012-11-23 at 22:44 -0500, simo wrote:
> On Sat, 2012-11-24 at 03:26 +0100, Jelmer Vernooij wrote:
> > On Sat, 2012-11-24 at 03:06 +0100, Jelmer Vernooij wrote:
> > > Hi Andreas,
> > > 
> > > On Fri, 2012-11-23 at 16:41 +0100, Andreas Schneider wrote:
> > > > The following patchset introduces a new function named samba_getpass so we can
> > > > get rid of the deprecated system getpass() function and the implementation
> > > > in libreplace. This way there should be no libreplace on Linux at all.
> > > This breaks the samba3 autoconf build:
> > > 
> > > lib/util_cmdline.o: In function `set_cmdline_auth_info_getpass':
> > > /memdisk/jelmer/a/b747076/samba3/source3/lib/util_cmdline.c:262:
> > > undefined reference to `samba_getpass'
> > > collect2: ld returned 1 exit status
> > > make: *** [bin/testparm] Error 1
> > > make: *** Waiting for unfinished jobs....
> > > lib/util_cmdline.o: In function `set_cmdline_auth_info_getpass':
> > > /memdisk/jelmer/a/b747076/samba3/source3/lib/util_cmdline.c:262:
> > > undefined reference to `samba_getpass'
> > > collect2: ld returned 1 exit status
> > > make: *** [bin/smbta-util] Error 1
> > > lib/util_cmdline.o: In function `set_cmdline_auth_info_getpass':
> > > /memdisk/jelmer/a/b747076/samba3/source3/lib/util_cmdline.c:262:
> > > undefined reference to `samba_getpass'
> > > collect2: ld returned 1 exit status
> > > make: *** [bin/smbcontrol] Error 1
> > > lib/util_cmdline.o: In function `set_cmdline_auth_info_getpass':
> > > /memdisk/jelmer/a/b747076/samba3/source3/lib/util_cmdline.c:262:
> > > undefined reference to `samba_getpass'
> > > collect2: ld returned 1 exit status
> > > make: *** [bin/nmblookup] Error 1
> > 
> > Incidentally, this is one of the reasons why I don't like reviewer-push.
> > If an autobuild fails, I'd rather have the original developer deal with
> > debugging the problem, and coming back to the reviewer with a request
> > for a follow-up review. Instead, this just makes reviews more
> > time-consuming (thus increasing the bar for reviews to happen).
> 
> If autobuild fails, you just send the author the logs, or tell him to
> check out why it failed. Is it really that hard to fwd an autobuild
> email ?

It's not just that. Compared to non-reviewer-push I ended up:

 * carefully apply all patches locally
 * add the appropriate reviewed-by tags to the commits
 * run autobuild
 * get e-mail back, look at logs
 * check I had applied all patches correctly
 * reproduce issue locally
 * send email to original author

Cheers,

Jelmer