How do do bulk reviews?
obnox at samba.org
Tue Jan 28 02:37:48 MST 2014
On 2014-01-28 at 18:55 +1100, Martin Schwenke wrote:
> On Mon, 27 Jan 2014 11:53:54 +0100, Michael Adam <obnox at samba.org>
> > I usually do reviews also of a bigger patchset like this:
> > 0. look over the whole patchset
> > 1. apply the whole patchset to a checkout of master
> > 2. check that it compiles and passes make test, etc
> > 3. rebase -i origin master and choose "edit" for each commit
> > 4. at each commit
> > - look at it thoroughly
> > - maybe (ideally) check it compiles
> > - do "git ci --amend", adding my reviewed-by tag
> > For adding the reviewed-by, I use the metze's macros for vim
> > (see https://wiki.samba.org/index.php/CodeReview#.7E.2F.vimrc)
> > and only type "R" in the editor.
> > I think it does not harm to touch each commit message
> > manually, also checking the message itself.
> > IMHO, this is what is required to do serious and not superficial
> > reviews.
> Sorry, that's just not logical. :-(
> You can do all of the actual review steps in 0, 1, 2, 4 and still add
> the "Reviewed-by" tags automatically. Doing the latter does not
> imply that you have missed any of the actual review steps or that you
> have done any of them superficially.
Yeah, but my point is that when you do rebase -i and
look at each patch individually anyways, then reviewing
the commit message as part of commit --amend, it is just
one additional keystroke to add your review tag: This
no real extra effort. But the advantage I see is that
it is done conciously and not automatically.
Also, I do of cours not care at all how --technically-- the
review-tag is added. But I think it is very important to
review each patch individually. And the natural too do this
is "git rebase -i".
Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 215 bytes
Desc: Digital signature
More information about the samba-technical