Re: Code review [Was: GNA is down...}

[Sebastian Reitenbach]

On Tuesday, February 14, 2012 12:49 CET, David Chisnall <theraven@sucs.org> 
wrote:

> On 14 Feb 2012, at 08:43, Sebastian Reitenbach wrote:
>
> > Code review is usually done before its commited to the main tree. For each 
> > part of the tree, there is at least one, sometimes more maintainers.
> > If you have changes, you figure out, who is the maintainer, and send the 
> > patch for review.
>
> There are better tools for this.  For a while for Étoilé Nicolas ran 
> ReviewBoard[1], which let you upload a diff and let other people inspect it 
> against the current svn head.  LLVM has a mechanism that works the other way 
> and scrapes the llvm-commits mailing list for patches as attachments and 
> presents them in a web interface (I'm not sure what this uses, but I could 
> find out).
>
> For post-commit reviews, pretty much anything supports showing the diff in a 
> convenient way.  I usually look at GNUstep changes using viewvc on GNA, which 
> lets you inspect a revision, see what has changed, and inspect diffs for 
> everything.  Fossil has a similar functionality built in (and, because the 
> web UI can run locally, you can see the same interface whether connected or 
> disconnected).   Github has a version that reflects the git philosophy: more 
> features, worse UI.

Yeah, there are probably better ways of reviewing code changes prior commit. 
This is just to illustrate, how we do it there.
The main point I intended was just with the code review before commit. How its 
done, doesn't really matter. If the VCS has something built-in, or there are 
good tools available that help in that process, even better. I wasn' t aware of 
the ReviewBoard.
But it sounds, you dropped using it, why, the process is still too cumbersome?

>
> > Important here is to send a patch inline, not as attachment.
>
> The down side of this is that mail clients and mailing list software have a 
> habit of mangling diffs sent inline, so you often can't apply a diff sent 
> this way.  You can do code review, but not testing.

Yep, I also had to learn that the hard way. I stopped using thunderbird in 
favor of using SOGo web interface, since I did not got Thunderbird to not to 
mangle inline diffs :(
But since then, I never had problems anymore with applying diffs from others, 
nor that others had problems with my diffs.

Sebastian

>
> David
>
> [1] http://www.reviewboard.org/
>
> -- Sent from my Difference Engine