How do do bulk reviews?

[Michael Adam]

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>
> wrote:
> 
> > 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...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140128/7c8e8bf6/attachment.pgp>