Re: [webkit-dev] to reitveld or not to reitveld

[Jeremy Orlow]

On Thu, Jun 11, 2009 at 5:19 PM, Jeremy Orlow  wrote:

> On Thu, Jun 11, 2009 at 4:50 PM, Joe Mason wrote:
>
>> Mark Rowe wrote:
>>
>>>
>>> On 2009-06-11, at 15:16, Ojan Vafai wrote:
>>>
>>>  On Sat, Jun 6, 2009 at 7:14 PM, Ojan Vafai >>> o...@google.com>> wrote:

On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe >>>> wrote:

There are also concerns about access to the data store of the
application, backup procedures, etc.  Our existing servers
are well understood in this regard.   We've also found in the
past that having services spread across different systems
causes confusion when something goes wrong, for whatever
reason, as it's not clear who to contact to address the problem.


For what it's worth, we've had next to zero maintenance effort go
into keeping rietveld running on appengine. As far as I know,
it's been pretty much stable and problem free. But I don't
actually maintain it, so I can't say that for sure. :)

 It seems to me that all the issues raised with using reitveld are
 solvable. Assuming you all are OK with doing this iteratively instead of
 needing all the issues to be resolved on day one, I think we can probably
 start taking concrete steps forward.

>>>
>>> Given what has been said so far I'm still not clear why Rietvald is a
>>> better option than Review Board.
>>>
>> Well, I haven't heard anything concrete on why Review Board is better than
>> rveld, either.  All I've seen are some posts saying, "You know, Review Board
>> exists too." Is the UI better?  Is the architecture better?  Is the design
>> very different?  Is it easier to integrate with git?  Exactly how much work
>> is involved in hosting each of these solutions?  The only thing concrete
>> I've seen is that Review Board is self-hosting, while rveld is tied to
>> AppEngine.
>
>
> ...and there's some that you could run rietveld without AppEngine
> infastructure if necessary.
>

er...there's "there's some _evidence_ that you could run rietveld without
AppEngine infastructure if necessary."  (referring back to Ojan's comment)


>
> FWIW, another project I (used to) work on tried Review Board out about a
> year ago.  We found it to be pretty clunky, so we continued just doing code
> reviews via diffs in email.  Maybe it's improved since then.  Either way, I
> believe the team is now using Rietveld for large reviews.
>
> J
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Jeremy Orlow]

On Thu, Jun 11, 2009 at 4:50 PM, Joe Mason wrote:

> Mark Rowe wrote:
>
>>
>> On 2009-06-11, at 15:16, Ojan Vafai wrote:
>>
>>  On Sat, Jun 6, 2009 at 7:14 PM, Ojan Vafai >> o...@google.com>> wrote:
>>>
>>>On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe >>> wrote:
>>>
>>>There are also concerns about access to the data store of the
>>>application, backup procedures, etc.  Our existing servers
>>>are well understood in this regard.   We've also found in the
>>>past that having services spread across different systems
>>>causes confusion when something goes wrong, for whatever
>>>reason, as it's not clear who to contact to address the problem.
>>>
>>>
>>>For what it's worth, we've had next to zero maintenance effort go
>>>into keeping rietveld running on appengine. As far as I know,
>>>it's been pretty much stable and problem free. But I don't
>>>actually maintain it, so I can't say that for sure. :)
>>>
>>> It seems to me that all the issues raised with using reitveld are
>>> solvable. Assuming you all are OK with doing this iteratively instead of
>>> needing all the issues to be resolved on day one, I think we can probably
>>> start taking concrete steps forward.
>>>
>>
>> Given what has been said so far I'm still not clear why Rietvald is a
>> better option than Review Board.
>>
> Well, I haven't heard anything concrete on why Review Board is better than
> rveld, either.  All I've seen are some posts saying, "You know, Review Board
> exists too." Is the UI better?  Is the architecture better?  Is the design
> very different?  Is it easier to integrate with git?  Exactly how much work
> is involved in hosting each of these solutions?  The only thing concrete
> I've seen is that Review Board is self-hosting, while rveld is tied to
> AppEngine.


...and there's some that you could run rietveld without AppEngine
infastructure if necessary.

FWIW, another project I (used to) work on tried Review Board out about a
year ago.  We found it to be pretty clunky, so we continued just doing code
reviews via diffs in email.  Maybe it's improved since then.  Either way, I
believe the team is now using Rietveld for large reviews.

J
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Joe Mason]

Mark Rowe wrote:


On 2009-06-11, at 15:16, Ojan Vafai wrote:

On Sat, Jun 6, 2009 at 7:14 PM, Ojan Vafai > wrote:


On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe mailto:mr...@apple.com>> wrote:

There are also concerns about access to the data store of the
application, backup procedures, etc.  Our existing servers
are well understood in this regard.   We've also found in the
past that having services spread across different systems
causes confusion when something goes wrong, for whatever
reason, as it's not clear who to contact to address the problem.


For what it's worth, we've had next to zero maintenance effort go
into keeping rietveld running on appengine. As far as I know,
it's been pretty much stable and problem free. But I don't
actually maintain it, so I can't say that for sure. :) 



It seems to me that all the issues raised with using reitveld are 
solvable. Assuming you all are OK with doing this iteratively instead 
of needing all the issues to be resolved on day one, I think we can 
probably start taking concrete steps forward.


Given what has been said so far I'm still not clear why Rietvald is a 
better option than Review Board.
Well, I haven't heard anything concrete on why Review Board is better 
than rveld, either.  All I've seen are some posts saying, "You know, 
Review Board exists too." Is the UI better?  Is the architecture 
better?  Is the design very different?  Is it easier to integrate with 
git?  Exactly how much work is involved in hosting each of these 
solutions?  The only thing concrete I've seen is that Review Board is 
self-hosting, while rveld is tied to AppEngine.


Joe
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Peter Kasting]

On Thu, Jun 11, 2009 at 3:21 PM, Mark Rowe  wrote:

> Given what has been said so far I'm still not clear why Rietvald is a
> better option than Review Board.
>

I think it's because we have a bunch of people who are (a) familiar with
using it and/or (b) work on it and can fix any problems, whereas neither is
true for Review Board.

Also I haven't really heard much significantly "Review Board > Rietveld",
more the sense of "we could try these both out".

It seems like reading between the lines you're really uncomfortable with
Rietveld in principle.  Is there something about it that you feel is
inherently not going to work well?

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Mark Rowe]

On 2009-06-11, at 15:16, Ojan Vafai wrote:


On Sat, Jun 6, 2009 at 7:14 PM, Ojan Vafai  wrote:
On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe  wrote:
There are also concerns about access to the data store of the  
application, backup procedures, etc.  Our existing servers are well  
understood in this regard.   We've also found in the past that  
having services spread across different systems causes confusion  
when something goes wrong, for whatever reason, as it's not clear  
who to contact to address the problem.


For what it's worth, we've had next to zero maintenance effort go  
into keeping rietveld running on appengine. As far as I know, it's  
been pretty much stable and problem free. But I don't actually  
maintain it, so I can't say that for sure. :)


It seems to me that all the issues raised with using reitveld are  
solvable. Assuming you all are OK with doing this iteratively  
instead of needing all the issues to be resolved on day one, I think  
we can probably start taking concrete steps forward.


Given what has been said so far I'm still not clear why Rietvald is a  
better option than Review Board.


- Mark



smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Ojan Vafai]

On Sat, Jun 6, 2009 at 7:14 PM, Ojan Vafai  wrote:

> On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe  wrote:
>
>> There are also concerns about access to the data store of the application,
>> backup procedures, etc.  Our existing servers are well understood in this
>> regard.   We've also found in the past that having services spread across
>> different systems causes confusion when something goes wrong, for whatever
>> reason, as it's not clear who to contact to address the problem.
>>
>
> For what it's worth, we've had next to zero maintenance effort go into
> keeping rietveld running on appengine. As far as I know, it's been pretty
> much stable and problem free. But I don't actually maintain it, so I can't
> say that for sure. :)
>

It seems to me that all the issues raised with using reitveld are solvable.
Assuming you all are OK with doing this iteratively instead of needing all
the issues to be resolved on day one, I think we can probably start taking
concrete steps forward.

Maintenance/hosting is the biggest unanswered question. Would hosting this
on reitveld (which does it's own replication and backups) be a deal-breaker?
If so, there are a couple options a quick search brought up. One is
http://arachnid.github.com/bdbdatastore/, which is linked to from the
AppEngine blog as a way of hosting appengine apps on your own hardware.
Another is
http://code.google.com/appengine/articles/gae_backup_and_restore.html, which
is a backup option for appengine.

I'll reiterate that we've had great uptime and reliability for the Chromium
reitveld instance on appengine. So, to me, hosting it yourself seems like
extra work without much real-world benefit.

Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[John Abd-El-Malek]

On Wed, Jun 10, 2009 at 8:12 AM, Joe Mason wrote:

> Mark Rowe wrote:
>
>> - UI is intimidating to newcomers.  This is clearly subjective, but since
>> the goal here is to make the review process friendlier, especially for new
>> contributors, I think it deserves calling out.
>>
> FWIW, after playing around with it for a few minutes I find its UI much,
> much friendlier than Bugzilla's itself.
>
> However, add me to the list for "not unless we can get seamless integration
> with the bug tracker and source control".  I think the biggest confusion for
> newcomers would be keeping track of the difference between the three tools,
> not learning how to use each one.  It doesn't matter to me whether this is
> achieved by actual deep integration with Bugzilla, or just passing data back
> and forth, as long as it feels like deep integration to the user.
>
> As a wild speculation: how hard would it be to build a new bug tracker by
> adding fields to Rietveld's issue descriptions, rather than trying to make
> changes to Bugzilla?  (A new bug report would simply be an issue without any
> patches uploaded yet.  We would need a space for general comments not tied
> to a line of code.  We'd need more metadata about each issue (component,
> priority, etc) and probably some new search and summary screens.  What else
> would be needed?)
>
> Add a CON for Rietveld: there's a surprising amount of overlap between
> computer terminology and the Rietveld method of crystallography, making it
> hard to sort out google results when looking for reviews of it.  Does
> anybody know of any unbiased third-party user stories that might help us
> evaluate the tool?
>
> Ojan or others who know the tool - can you explain a bit more about how it
> integrates with svn?


We use a script with Rietveld, gcl.py, that handles changelists.  It allows
you to have multiple changes on the same repository, each identified by a
name.  The script automates creation of a Rietveld issue when you first
upload.  It handles things like copied/moved/deleted/image files.


> I was under the impression that you just attach a patch, which could be
> from any source, but browsing
> http://code.google.com/p/rietveld/issues/list shows a few bug reports
> about svn integration that make it seem Rietveld is pulling more data from
> it (for instance, issue 82: ignores files created by svn cp, issue 100: fix
> for upload.py and svn with externals, and of course the eloquent issue 117:
> support cvs).  Would git integration mainly be a matter of making sure it
> parses git's patch output correctly?


>  Subjective, less important issues:
>> - I'm not sure about keeping patches and the bugs that they address in
>> separate systems.  It seems that discussion about a bug can end up split
>> between the two systems.
>>
> I don't think that's a minor issue at all.
>
>> - It's hard to spell.  Retyping it to fix the spelling makes me sad.
>>
> Agreed.  I expect will all end up calling it rfield soon enough (and I even
> typed that as rfiled the first time).
>
>> Ojan also mentioned ReviewBoard in his original email.  I've used it only
>> briefly, but I do know that it addresses some, but not all, of the issues
>> above (It's not tied to AppEngine, it works with both Subversion and Git,
>> and has some support for external authentication mechanisms).  It may
>> address others, but I've not looked closely enough to know for sure.
>>
> I'd like to hear some more comments on other code review systems.  Does
> anyone have any more in-depth comparisons with rfield?  Do they all use
> basically the same methodology with slightly different UI's, or are there
> major differences in approach?
>
> Joe
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Joe Mason]

Joe Mason wrote:
Agreed.  I expect will all end up calling it rfield soon enough (and I 
even typed that as rfiled the first time).
I mean rveld, of course.  I've been rereading Dracula, I think it's 
affecting my brain...


Joe
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Joe Mason]

Mark Rowe wrote:
- UI is intimidating to newcomers.  This is clearly subjective, but 
since the goal here is to make the review process friendlier, 
especially for new contributors, I think it deserves calling out.
FWIW, after playing around with it for a few minutes I find its UI much, 
much friendlier than Bugzilla's itself.


However, add me to the list for "not unless we can get seamless 
integration with the bug tracker and source control".  I think the 
biggest confusion for newcomers would be keeping track of the difference 
between the three tools, not learning how to use each one.  It doesn't 
matter to me whether this is achieved by actual deep integration with 
Bugzilla, or just passing data back and forth, as long as it feels like 
deep integration to the user.


As a wild speculation: how hard would it be to build a new bug tracker 
by adding fields to Rietveld's issue descriptions, rather than trying to 
make changes to Bugzilla?  (A new bug report would simply be an issue 
without any patches uploaded yet.  We would need a space for general 
comments not tied to a line of code.  We'd need more metadata about each 
issue (component, priority, etc) and probably some new search and 
summary screens.  What else would be needed?)


Add a CON for Rietveld: there's a surprising amount of overlap between 
computer terminology and the Rietveld method of crystallography, making 
it hard to sort out google results when looking for reviews of it.  Does 
anybody know of any unbiased third-party user stories that might help us 
evaluate the tool?


Ojan or others who know the tool - can you explain a bit more about how 
it integrates with svn?  I was under the impression that you just attach 
a patch, which could be from any source, but browsing 
http://code.google.com/p/rietveld/issues/list shows a few bug reports 
about svn integration that make it seem Rietveld is pulling more data 
from it (for instance, issue 82: ignores files created by svn cp, issue 
100: fix for upload.py and svn with externals, and of course the 
eloquent issue 117: support cvs).  Would git integration mainly be a 
matter of making sure it parses git's patch output correctly?

Subjective, less important issues:
- I'm not sure about keeping patches and the bugs that they address in 
separate systems.  It seems that discussion about a bug can end up 
split between the two systems.

I don't think that's a minor issue at all.

- It's hard to spell.  Retyping it to fix the spelling makes me sad.
Agreed.  I expect will all end up calling it rfield soon enough (and I 
even typed that as rfiled the first time).
Ojan also mentioned ReviewBoard in his original email.  I've used it 
only briefly, but I do know that it addresses some, but not all, of 
the issues above (It's not tied to AppEngine, it works with both 
Subversion and Git, and has some support for external authentication 
mechanisms).  It may address others, but I've not looked closely 
enough to know for sure.
I'd like to hear some more comments on other code review systems.  Does 
anyone have any more in-depth comparisons with rfield?  Do they all use 
basically the same methodology with slightly different UI's, or are 
there major differences in approach?


Joe

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[John Abd-El-Malek]

On Mon, Jun 8, 2009 at 5:15 PM, Eric Seidel  wrote:

> At least one person has tried to tie review-board with bugzilla:
>
> http://www.visophyte.org/blog/2009/03/20/using-review-board-for-bugzilla-request-queues-reviews/
>
> I expect that we'd have to hack review board to do bugzilla-based
> authentication.  (Just like we'd have to hack rietveld to not run on
> AppEngine if we used it.)
>
>
> I agree with others that the rietveld authentication being tied to
> AppEngine and thus Google accounts is a show-stopper for WebKit.  I
> have no interest in creating yet another account and then worrying
> about whether I'm correclty logged into both bugzilla and webkit.org's
> google account in order to do patch reviews.  Currently I avoid
> dealing with chromium's tracker/review tool because every time I do I
> have to log out of my gmail.com google account so that I can log into
> my chromium.org google account. :(


AppEngine apps don't have to use Google Accounts for authentication.
 Rietveld has its own user-account layer, so it's possible to plug-in
different forms of authentication.


>
>
> -eric
>
> On Sat, Jun 6, 2009 at 7:44 PM, Ojan Vafai wrote:
> > On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe  wrote:
> >>
> >> On 2009-06-06, at 15:02, Peter Kasting wrote:
> >>
> >> On Sat, Jun 6, 2009 at 1:48 AM, Mark Rowe  wrote:
> >>>
> >>> Of the issues that Ojan mentioned in his original email, I see three
> that
> >>> would need to be addressed before we could consider adopting Rietveld:
> >>>
> >>> - Currently tied to AppEngine.
> >>
> >> I don't understand why this is problematic in the least, any more than
> >> saying "Bugzilla is currently tied to being run by Bugzilla".  Why does
> it
> >> matter what the backing implementation of Rietveld is?
> >>
> >> Primarily due to the two points that you trimmed from my email:
> >>
> >> Two other major issues jump out at me:
> >> - Authentication. This is related to the AppEngine tie-in.
> >> - Authorization.  Patch reviews need to reflect the access controls on
> the
> >> bugs that they are associated with.
> >>
> >> There are also concerns about access to the data store of the
> application,
> >> backup procedures, etc.  Our existing servers are well understood in
> this
> >> regard.   We've also found in the past that having services spread
> across
> >> different systems causes confusion when something goes wrong, for
> whatever
> >> reason, as it's not clear who to contact to address the problem.
> >
> > As I see it, these are the only non-trivial issues with using
> > rietveld. Things like not working with git are trivial fixes (e.g. git
> adds
> > "a/" and "b/" to the paths in the diff that need to be ignored). Also, I
> > really don't believe the intimidating UI is a problem in practice.
> Newcomers
> > get used to it very quickly.
> > I don't know enough about the rietveld code or appengine to say how
> > difficult it would be to address the authentication/authorization issues.
> > These would be the reason's to consider something like review-board
> instead.
> > My guess is that the access control bit is doable, but I think you'd
> > ultimately still need to sign in to AppEngine using a Google account.
> > For what it's worth, we've had next to zero maintenance effort go into
> > keeping rietveld running on appengine. As far as I know, it's been pretty
> > much stable and problem free. But I don't actually maintain it, so I
> can't
> > say that for sure. :)
> > Review-board would be considerably less effort than integrating something
> > directly into bugzilla. But rietveld would be less effort than
> review-board
> > if we can get the above issues addressed since there are a number of
> people
> > who already know the codebase willing to help out here.
> > It seems worth taking a look at how much work it would be to get
> > review-board setup and integrated with bugzilla.
> > Ojan
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >
> >
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Ojan Vafai]

On Sun, Jun 7, 2009 at 7:51 AM, Mark Rowe  wrote:

> On 2009-06-06, at 15:02, Peter Kasting wrote:
>
> On Sat, Jun 6, 2009 at 1:48 AM, Mark Rowe  wrote:
>
>> Of the issues that Ojan mentioned in his original email, I see three that
>> would need to be addressed before we could consider adopting Rietveld:
>>
>  - Currently tied to AppEngine.
>>
>
> I don't understand why this is problematic in the least, any more than
> saying "Bugzilla is currently tied to being run by Bugzilla".  Why does it
> matter what the backing implementation of Rietveld is?
>
> Primarily due to the two points that you trimmed from my email:
>
> Two other major issues jump out at me:
> - Authentication. This is related to the AppEngine tie-in.
> - Authorization.  Patch reviews need to reflect the access controls on the
> bugs that they are associated with.
>
> There are also concerns about access to the data store of the application,
> backup procedures, etc.  Our existing servers are well understood in this
> regard.   We've also found in the past that having services spread across
> different systems causes confusion when something goes wrong, for whatever
> reason, as it's not clear who to contact to address the problem.
>

As I see it, these are the only non-trivial issues with using
rietveld. Things like not working with git are trivial fixes (e.g. git adds
"a/" and "b/" to the paths in the diff that need to be ignored). Also, I
really don't believe the intimidating UI is a problem in practice. Newcomers
get used to it very quickly.

I don't know enough about the rietveld code or appengine to say how
difficult it would be to address the authentication/authorization issues.
These would be the reason's to consider something like review-board instead.
My guess is that the access control bit is doable, but I think you'd
ultimately still need to sign in to AppEngine using a Google account.

For what it's worth, we've had next to zero maintenance effort go into
keeping rietveld running on appengine. As far as I know, it's been pretty
much stable and problem free. But I don't actually maintain it, so I can't
say that for sure. :)

Review-board would be considerably less effort than integrating something
directly into bugzilla. But rietveld would be less effort than review-board
if we can get the above issues addressed since there are a number of people
who already know the codebase willing to help out here.

It seems worth taking a look at how much work it would be to get
review-board setup and integrated with bugzilla.

Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Mark Rowe]

On 2009-06-06, at 15:02, Peter Kasting wrote:


On Sat, Jun 6, 2009 at 1:48 AM, Mark Rowe  wrote:
Per Ojan's original email it is not as simple as adding a few URLs  
to some scripts, code changes would be needed to make it suitable  
for our purposes.  Let's try and avoid hyperbole: it makes it  
difficult to have a reasonable discussion.


I'm not trying to be hyperbolic.  It is my sincere opinion that in  
fact the changes are as simple as I mentioned.


Of the issues that Ojan mentioned in his original email, I see three  
that would need to be addressed before we could consider adopting  
Rietveld:

- Currently tied to AppEngine.

I don't understand why this is problematic in the least, any more  
than saying "Bugzilla is currently tied to being run by Bugzilla".   
Why does it matter what the backing implementation of Rietveld is?


Primarily due to the two points that you trimmed from my email:


Two other major issues jump out at me:
- Authentication. This is related to the AppEngine tie-in.
- Authorization.  Patch reviews need to reflect the access controls  
on the bugs that they are associated with.


There are also concerns about access to the data store of the  
application, backup procedures, etc.  Our existing servers are well  
understood in this regard.   We've also found in the past that having  
services spread across different systems causes confusion when  
something goes wrong, for whatever reason, as it's not clear who to  
contact to address the problem.



- Doesn't work with diff's generated by git.

I didn't realize git was formally supported by WebKit.  I assume git  
can generate diff/patch/svn-compatible diffs with some options (I am  
not a git user).


It's not our official version control system, but many WebKit  
contributors use git with WebKit via git-svn.  Most of our scripts  
have been updated to work with both Subversion and Git, and something  
as central as patch review would definitely need to continue to  
support users of git.



- It's hard to spell.  Retyping it to fix the spelling makes me sad.

Other than this email series, I've never actually had to spell  
Rietveld.  Certainly not while submitting, editing, reviewing, or  
landing patches :).  I think this is irrelevant.


Great to see that we all have a sense of humor.

In summary, I did not realize that the WebKit community was even  
interested in changing their review system before seeing these  
emails, but if they are, I sincerely believe that Rietveld is far  
better than Bugzilla for patch review and strong consideration  
should be given to simply dropping it in, which I believe would be  
very easy to do.


I don't disagree that modifying an existing system to fit our needs  
may be easier than improving Bugzilla, but you're still overstating  
the ease with which Rietveld can be dropped in.


If we're all convinced that moving patch review to a separate system  
is a good idea, then we should also look further in to Review Board too.


- Mark



smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Peter Kasting]

Clarifications:

On Sat, Jun 6, 2009 at 3:02 PM, Peter Kasting  wrote:

> I'm not trying to be hyperbolic.
>

On the other hand, I could simply be flat-out wrong.  Although I did re-read
Ojan's email and I don't I see him as saying there'd be a lot of actual
coding needed to try Rietveld.

- Doesn't work with diff's generated by git.
>>
>
> I didn't realize git was formally supported by WebKit.  I assume git can
> generate diff/patch/svn-compatible diffs with some options (I am not a git
> user).
>

Also, a bunch of Chromium team members use git full-time, so I assume they
have in fact already solved this problem.

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Peter Kasting]

On Sat, Jun 6, 2009 at 1:48 AM, Mark Rowe  wrote:

> Per Ojan's original email it is not as simple as adding a few URLs to some
> scripts, code changes would be needed to make it suitable for our purposes.
>  Let's try and avoid hyperbole: it makes it difficult to have a reasonable
> discussion.
>

I'm not trying to be hyperbolic.  It is my sincere opinion that in fact the
changes are as simple as I mentioned.

Of the issues that Ojan mentioned in his original email, I see three that
> would need to be addressed before we could consider adopting Rietveld:
> - Currently tied to AppEngine.
>

I don't understand why this is problematic in the least, any more than
saying "Bugzilla is currently tied to being run by Bugzilla".  Why does it
matter what the backing implementation of Rietveld is?


> - Doesn't work with diff's generated by git.
>

I didn't realize git was formally supported by WebKit.  I assume git can
generate diff/patch/svn-compatible diffs with some options (I am not a git
user).

- UI is intimidating to newcomers.  This is clearly subjective, but since
> the goal here is to make the review process friendlier, especially for new
> contributors, I think it deserves calling out.
>

I find the Rietveld UI mostly easy to understand with one or two learning
bits.  It didn't intimidate me when I started using it.  It certainly was
far easier to understand than Bugzilla's review methods.  In my mind this is
a plus, not a minus.

Subjective, less important issues:
> - I'm not sure about keeping patches and the bugs that they address in
> separate systems.  It seems that discussion about a bug can end up split
> between the two systems.
>

This is not really much different from how right now bugs and IRC can both
serve as discussion points for an issue.  Generally in Chromium today bugs
get discussed on the bug, and patches get discussed on the patch.  If a
patch raises issues with a bug, we usually comment on the bug and note on
the patch as appropriate ("sorry, I don't think this bug should be
addressed.  See comments there.")


> - It's hard to spell.  Retyping it to fix the spelling makes me sad.
>

Other than this email series, I've never actually had to spell Rietveld.
 Certainly not while submitting, editing, reviewing, or landing patches :).
 I think this is irrelevant.

In summary, I did not realize that the WebKit community was even interested
in changing their review system before seeing these emails, but if they are,
I sincerely believe that Rietveld is far better than Bugzilla for patch
review and strong consideration should be given to simply dropping it in,
which I believe would be very easy to do.

PK

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Mark Rowe]

On 2009-06-06, at 00:17, Peter Kasting wrote:


On Fri, Jun 5, 2009 at 11:43 PM, Mark Rowe  wrote:
Dropping our existing practice of using Bugzilla for patch reviews  
is one way of addressing this.  Folding the more useful features of  
Rietveld in to Bugzilla to improve Bugzilla-based patch reviews is  
another.  We all seem to be in agreement that the tools involved  
with reviewing a patch have room for improvement, but I've not seen  
a compelling reason why the former is a better way forward.


If people were really interested in changing, then it would take  
probably two orders of magnitude less effort to set up a Rietveld  
instance to associate with Bugzilla (to the degree it currently can  
associate with the Google Code bug tracker), as compared with  
improving Bugzilla.  The former is basically adding a few URLs to  
some scripts, the latter is highly nontrivial coding.  That seems  
compelling to me.


Per Ojan's original email it is not as simple as adding a few URLs to  
some scripts, code changes would be needed to make it suitable for our  
purposes.  Let's try and avoid hyperbole: it makes it difficult to  
have a reasonable discussion.


Of the issues that Ojan mentioned in his original email, I see three  
that would need to be addressed before we could consider adopting  
Rietveld:

- Currently tied to AppEngine.
- Doesn't work with diff's generated by git.
- UI is intimidating to newcomers.  This is clearly subjective, but  
since the goal here is to make the review process friendlier,  
especially for new contributors, I think it deserves calling out.


Two other major issues jump out at me:
- Authentication. This is related to the AppEngine tie-in.
- Authorization.  Patch reviews need to reflect the access controls on  
the bugs that they are associated with.


Subjective, less important issues:
- I'm not sure about keeping patches and the bugs that they address in  
separate systems.  It seems that discussion about a bug can end up  
split between the two systems.

- It's hard to spell.  Retyping it to fix the spelling makes me sad.

Ojan also mentioned ReviewBoard in his original email.  I've used it  
only briefly, but I do know that it addresses some, but not all, of  
the issues above (It's not tied to AppEngine, it works with both  
Subversion and Git, and has some support for external authentication  
mechanisms).  It may address others, but I've not looked closely  
enough to know for sure.


(Right now it's about 10x easier for me to get a Chromium patch  
reviewed than a WebKit one just because a single shell command can  
create a Rietveld issue with my patch and set the description up  
for me.)


This something of a non-sequiter, since it is trivial to create a  
script to do the same with Bugzilla.  I've heard mentions of a git- 
send-bugzilla script that does most of this already, and I'm sure it  
could easily be adapted for those preferring SVN.


True.  Still, I _have_ that script for Chromium, and I don't for  
WebKit :).


In my experience doing something to address a problem tends to be a  
very effective method of making the problem go away, especially when  
that something is easy to do.


- Mark



smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Eric Seidel]

> This something of a non-sequiter, since it is trivial to create a script to
> do the same with Bugzilla.  I've heard mentions of a git-send-bugzilla
> script that does most of this already, and I'm sure it could easily be
> adapted for those preferring SVN.

https://bugs.webkit.org/show_bug.cgi?id=25603
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Peter Kasting]

On Fri, Jun 5, 2009 at 11:43 PM, Mark Rowe  wrote:

> Dropping our existing practice of using Bugzilla for patch reviews is one
> way of addressing this.  Folding the more useful features of Rietveld in to
> Bugzilla to improve Bugzilla-based patch reviews is another.  We all seem to
> be in agreement that the tools involved with reviewing a patch have room for
> improvement, but
> I've not seen a compelling reason why the former is a better way forward.
>

If people were really interested in changing, then it would take probably
two orders of magnitude less effort to set up a Rietveld instance to
associate with Bugzilla (to the degree it currently can associate with the
Google Code bug tracker), as compared with improving Bugzilla.  The former
is basically adding a few URLs to some scripts, the latter is highly
nontrivial coding.  That seems compelling to me.

> (Right now it's about 10x easier for me to get a Chromium patch reviewed
> than a WebKit one just because a single shell command can create a Rietveld
> issue with my patch and set the description up for me.)
>
>
> This something of a non-sequiter, since it is trivial to create a script to
> do the same with Bugzilla.  I've heard mentions of a git-send-bugzilla
> script that does most of this already, and I'm sure it could easily be
> adapted for those preferring SVN.
>

True.  Still, I _have_ that script for Chromium, and I don't for WebKit :).

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Mark Rowe]

On 2009-06-05, at 22:50, Peter Kasting wrote:

On Fri, Jun 5, 2009 at 9:13 PM, Darren VanBuren  
 wrote:
I agree that using RPC is inefficient, and that we don't want to  
make the review process any more of a pain. We could also try  
writing our own code review software specifically designed to work  
with Bugzilla, so that we could run directly in the Bugzilla  
environment, and we could modify and retrieve bugs without throwing  
stuff around RPC channels, just by running some calls in the  
Bugzilla modules.


FWIW, in Chromium land we do all the patches *solely* on Rietveld,  
and never touch the bug tracker at all with them.  We have tools  
that auto-update bugs when patches are checked in and can provide  
handy links back and forth between the tools, and that's enough.   
I'm not a WebKit reviewer but I was a Mozilla reviewer, which also  
does things on Bugzilla, and I don't miss the ability to post a  
patch on a bug at all.  There is literally nothing in that workflow  
that helps me review/land patches more easily, and it's still just  
as easy to backtrack after the fact and find what got reviewed/ 
landed starting from a bug. So if people who wanted to use Rietveld  
to do code review didn't have obvious ways to attach those patches  
to Bugzilla bugs, I'm not sure it would be a big stumbling block.


I'm a fan of simplicity, and of having one obvious way to do  
something.  Having two different ways to put a patch up for review is  
at odds with this, so it's not a solution that I would be happy with.   
Dropping our existing practice of using Bugzilla for patch reviews is  
one way of addressing this.  Folding the more useful features of  
Rietveld in to Bugzilla to improve Bugzilla-based patch reviews is  
another.  We all seem to be in agreement that the tools involved with  
reviewing a patch have room for improvement, but I've not seen a  
compelling reason why the former is a better way forward.


(Right now it's about 10x easier for me to get a Chromium patch  
reviewed than a WebKit one just because a single shell command can  
create a Rietveld issue with my patch and set the description up for  
me.)


This something of a non-sequiter, since it is trivial to create a  
script to do the same with Bugzilla.  I've heard mentions of a git- 
send-bugzilla script that does most of this already, and I'm sure it  
could easily be adapted for those preferring SVN.


- Mark



smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Peter Kasting]

On Fri, Jun 5, 2009 at 9:13 PM, Darren VanBuren  wrote:

> I agree that using RPC is inefficient, and that we don't want to make the
> review process any more of a pain. We could also try writing our own code
> review software specifically designed to work with Bugzilla, so that we
> could run directly in the Bugzilla environment, and we could modify and
> retrieve bugs without throwing stuff around RPC channels, just by running
> some calls in the Bugzilla modules.
>

FWIW, in Chromium land we do all the patches *solely* on Rietveld, and never
touch the bug tracker at all with them.  We have tools that auto-update bugs
when patches are checked in and can provide handy links back and forth
between the tools, and that's enough.  I'm not a WebKit reviewer but I was a
Mozilla reviewer, which also does things on Bugzilla, and I don't miss the
ability to post a patch on a bug at all.  There is literally nothing in that
workflow that helps me review/land patches more easily, and it's still just
as easy to backtrack after the fact and find what got reviewed/landed
starting from a bug.  So if people who wanted to use Rietveld to do code
review didn't have obvious ways to attach those patches to Bugzilla bugs,
I'm not sure it would be a big stumbling block.  (Right now it's about 10x
easier for me to get a Chromium patch reviewed than a WebKit one just
because a single shell command can create a Rietveld issue with my patch and
set the description up for me.)

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Darren VanBuren]

I agree that using RPC is inefficient, and that we don't want to make  
the review process any more of a pain. We could also try writing our  
own code review software specifically designed to work with Bugzilla,  
so that we could run directly in the Bugzilla environment, and we  
could modify and retrieve bugs without throwing stuff around RPC  
channels, just by running some calls in the Bugzilla modules.


Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 8:48 PM, Mark Rowe wrote:



On 2009-06-05, at 19:02, Darren VanBuren wrote:

Surprisingly, the bug isn't a duplicate, or if there is a dupe, it  
isn't filed correctly.


But I agree that any code review tool should be integrated with  
bugs.webkit.org, otherwise there would be a huge disorganized mess  
and it wouldn't be any better.


Bugzilla wouldn't be hard to extend for this purpose, just adding a  
field for review status and then making whatever code review tool  
you chose update Bugzilla solves (b).


Some modifications in the tool could also make it attach the  
patches to a bug, and you could also update any field in the bug.


I mean, retiveld seems like a wonderful tool, it seems like  
something that would extend Bugzilla quite nicely. Pushing data to  
Bugzilla can simply be done with XML-RPC according to this page on  
bugzilla.org: http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html 
 and there's plenty of XML libraries for Python you could use to  
work over XML-RPC.


I don't think that duplicating data in two systems and pushing it  
back and forth via an RPC mechanism is what Dave had in mind when he  
was speaking of tight integration.  We need to *streamline* the  
process of uploading and reviewing a patch, and having two different  
ways to do this seems like a large step in the opposite direction.


- Mark





PGP.sig
Description: This is a digitally signed message part
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[Mark Rowe]

On 2009-06-05, at 19:02, Darren VanBuren wrote:

Surprisingly, the bug isn't a duplicate, or if there is a dupe, it  
isn't filed correctly.


But I agree that any code review tool should be integrated with bugs.webkit.org 
, otherwise there would be a huge disorganized mess and it wouldn't  
be any better.


Bugzilla wouldn't be hard to extend for this purpose, just adding a  
field for review status and then making whatever code review tool  
you chose update Bugzilla solves (b).


Some modifications in the tool could also make it attach the patches  
to a bug, and you could also update any field in the bug.


I mean, retiveld seems like a wonderful tool, it seems like  
something that would extend Bugzilla quite nicely. Pushing data to  
Bugzilla can simply be done with XML-RPC according to this page on bugzilla.org 
: http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html 
 and there's plenty of XML libraries for Python you could use to  
work over XML-RPC.


I don't think that duplicating data in two systems and pushing it back  
and forth via an RPC mechanism is what Dave had in mind when he was  
speaking of tight integration.  We need to *streamline* the process of  
uploading and reviewing a patch, and having two different ways to do  
this seems like a large step in the opposite direction.


- Mark



smime.p7s
Description: S/MIME cryptographic signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] to reitveld or not to reitveld

[David Kilzer]

On Friday, June 5, 2009 Ojan Vafai wrote:
> This is what I meant by "light-weight" integration. All the review
> information would be reflected in the bugzilla bug. You would never
> be required to use reitveld for anything.

But I'm a reviewer.  Don't you want to be selling me on the virtues of 
Reitveld?  :)

What happens if I choose to update a patch in Bugzilla instead of reitveld 
(assuming I'm not required to use Reitveld as you say)?  Will Bugzilla push the 
status of the patch back to Reitveld?

> A review tool like reitveld is quite a bit of work. Adding 
> similar functionality to bugzilla itself is a non-trivial amount
> of work. I don't see what integrating this functionality any
> more tightly into bugzilla buys us that is worth the order(s) of
> magnitude more effort that approach would take.

The one major thing it would buy us is less maintenance--adding another web 
site would double the amount of maintenance for the bug system.  I can easily 
imagine that upgrading one would break integration with the other and 
vice-versa.  Is there something I'm missing that would mitigate the risk of 
additional maintenance and break-on-upgrade issues?

It's obvious that a lot of work has gone into Reitveld, and I'm sure it's a 
great tool.  I just think it's a shame that no one has stepped up to provide 
similar functionality for Bugzilla, thereby improving the status quo for all 
users of this popular open source tool.

Dave





From: Ojan Vafai 
To: Darren VanBuren 
Cc: David Kilzer ; Jeremy Orlow ; 
WebKit Development 
Sent: Friday, June 5, 2009 7:25:40 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld


This is what I meant by "light-weight" integration. All the review information 
would be reflected in the bugzilla bug. You would never be required to use 
reitveld for anything.

We would be able to:
1. Add a link to bugzilla that would take you to the reitveld code review and 
upload the patch to reitveld if it hasn't been uploaded already.
2. Have all comments published in reitveld be posted to the bug.
3. Have checkboxes in reitveld for r+, r- that would update bugzilla.
4. I think we can even have comments made directly to bugzilla be reflected in 
reitveld by having a bot that monitors bugzilla update emails.

A review tool like reitveld is quite a bit of work. Adding similar 
functionality to bugzilla itself is a non-trivial amount of work. I don't see 
what integrating this functionality any more tightly into bugzilla buys us that 
is worth the order(s) of magnitude more effort that approach would take.

Ojan

On Sat, Jun 6, 2009 at 11:02 AM, Darren VanBuren  wrote:

Surprisingly, the bug isn't a duplicate, or if there is a dupe, it isn't filed 
correctly.

But I agree that any code review tool should be integrated with 
bugs.webkit.org, otherwise there would be a huge disorganized mess and it 
wouldn't be any better.

Bugzilla wouldn't be hard to extend for this purpose, just adding a field for 
review status and then making whatever code review tool you chose update 
Bugzilla solves (b).

Some modifications in the tool could also make it attach the patches to a bug, 
and you could also update any field in the bug.

I mean, retiveld seems like a wonderful tool, it seems like something that 
would extend Bugzilla quite nicely. Pushing data to Bugzilla can simply be done 
with XML-RPC according to this page on bugzilla.org: 
http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html
 and there's plenty of XML libraries for Python you could use to work over 
XML-RPC.

Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/


On Jun 5, 2009, at 6:21 PM, David Kilzer wrote:

I think this is a great direction to move in, but (IMO) any such tool should be 
tightly integrated with bugs.webkit.org so that (a) you don't have to post the 
same patch more than once, (b) the review status of the patch is visible in 
bugs.webkit.org without clicking on a link and (c) it's easy to switch between 
reviewing the patch and updating the bug itself.

I just filed a Bugzilla bug about adding such a feature to Bugzilla itself 
(although I wouldn't be surprised if it's a dupe):

Bugzilla needs better patch review process with annotations and versioned 
patches
<https://bugzilla.mozilla.org/show_bug.cgi?id=496670>

Dave




________________________
From: Jeremy Orlow 
To: Ojan Vafai 
Cc: WebKit Development 
Sent: Friday, June 5, 2009 5:08:47 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld

For what it's worth, I definitely think a tool like reitveld would help the 
code review process.  Inline comments and more context than the couple lines 
the diff provides are really, really helpful.


On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:

Sorry i

Re: [webkit-dev] to reitveld or not to reitveld

[Darren VanBuren]

Apparently, according to http://code.google.com/p/rietveld/, we've  
been spelling rietveld wrong.

Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 7:25 PM, Ojan Vafai wrote:

This is what I meant by "light-weight" integration. All the review  
information would be reflected in the bugzilla bug. You would never  
be required to use reitveld for anything.


We would be able to:
1. Add a link to bugzilla that would take you to the reitveld code  
review and upload the patch to reitveld if it hasn't been uploaded  
already.

2. Have all comments published in reitveld be posted to the bug.
3. Have checkboxes in reitveld for r+, r- that would update bugzilla.
4. I think we can even have comments made directly to bugzilla be  
reflected in reitveld by having a bot that monitors bugzilla update  
emails.


A review tool like reitveld is quite a bit of work. Adding similar  
functionality to bugzilla itself is a non-trivial amount of work. I  
don't see what integrating this functionality any more tightly into  
bugzilla buys us that is worth the order(s) of magnitude more effort  
that approach would take.


Ojan

On Sat, Jun 6, 2009 at 11:02 AM, Darren VanBuren  
 wrote:
Surprisingly, the bug isn't a duplicate, or if there is a dupe, it  
isn't filed correctly.


But I agree that any code review tool should be integrated with  
bugs.webkit.org, otherwise there would be a huge disorganized mess  
and it wouldn't be any better.


Bugzilla wouldn't be hard to extend for this purpose, just adding a  
field for review status and then making whatever code review tool  
you chose update Bugzilla solves (b).


Some modifications in the tool could also make it attach the patches  
to a bug, and you could also update any field in the bug.


I mean, retiveld seems like a wonderful tool, it seems like  
something that would extend Bugzilla quite nicely. Pushing data to  
Bugzilla can simply be done with XML-RPC according to this page on  
bugzilla.org: http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html 
 and there's plenty of XML libraries for Python you could use to  
work over XML-RPC.


Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 6:21 PM, David Kilzer wrote:

I think this is a great direction to move in, but (IMO) any such  
tool should be tightly integrated with bugs.webkit.org so that (a)  
you don't have to post the same patch more than once, (b) the  
review status of the patch is visible in bugs.webkit.org without  
clicking on a link and (c) it's easy to switch between reviewing  
the patch and updating the bug itself.


I just filed a Bugzilla bug about adding such a feature to Bugzilla  
itself (although I wouldn't be surprised if it's a dupe):


Bugzilla needs better patch review process with annotations and  
versioned patches

<https://bugzilla.mozilla.org/show_bug.cgi?id=496670>

Dave


From: Jeremy Orlow 
To: Ojan Vafai 
Cc: WebKit Development 
Sent: Friday, June 5, 2009 5:08:47 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld

For what it's worth, I definitely think a tool like reitveld would  
help the code review process.  Inline comments and more context  
than the couple lines the diff provides are really, really helpful.


On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:
Sorry in advance for the long email. I'm trying to be thorough.

There's been a lot of discussion on #webkit about possibly using a  
code review tool like reitveld for webkit reviews. There's been  
various suggestions and a few misunderstandings, so it seems worth  
having a more formal discussion about this with the larger WebKit  
community.


The things I'd like to assess here are:
1. Pros/Cons of using a system like reitveld. I listed some below.  
Please add any that I missed.
2. Whether the WebKit community is interested in pursuing something  
like this.

3. If there is interest, what is the best way to move forward.

WHAT IS REITVELD
It's a code review tool. Reitveld doesn't allow you to do anything  
that is impossible with the current review process, however, it  
makes the review process more efficient and less error-prone. As  
such, it makes it easier and less time-consuming to do good,  
thorough code reviews.


The basic gist of reitveld is that it allows you to put comments  
inline and send them all in one chunk. Then it lets the reviewee  
easily respond to each comment individually and send all the  
responses in one chunk.


EXAMPLE CHROMIUM PATCH
http://codereview.chromium.org/119103

Note that you can view the patch in each version that was uploaded  
and that you can diff between versions. Also, if a comment was made  
in the version you were looking at, then you can see all the  
comments/responses.


To see this nicely, under "Delta from patch

Re: [webkit-dev] to reitveld or not to reitveld

[Darren VanBuren]

1. Avoiding uploading a patch twice is always nice. I think either  
reitveld should upload attachments when they're attached to bugs on  
bugzilla, as long as the title matches something like +reitveld

2. I guess I missed saying that.
3. David mentioned that feature.
4. A bot to monitor bugzilla emails would be extremely easy, and it  
gets updates very quickly (not more than 1 minute, as far as I know,  
but especially quickly if the email address was also on the machine  
that runs bugs.webkit.org)


Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 7:25 PM, Ojan Vafai wrote:

This is what I meant by "light-weight" integration. All the review  
information would be reflected in the bugzilla bug. You would never  
be required to use reitveld for anything.


We would be able to:
1. Add a link to bugzilla that would take you to the reitveld code  
review and upload the patch to reitveld if it hasn't been uploaded  
already.

2. Have all comments published in reitveld be posted to the bug.
3. Have checkboxes in reitveld for r+, r- that would update bugzilla.
4. I think we can even have comments made directly to bugzilla be  
reflected in reitveld by having a bot that monitors bugzilla update  
emails.


A review tool like reitveld is quite a bit of work. Adding similar  
functionality to bugzilla itself is a non-trivial amount of work. I  
don't see what integrating this functionality any more tightly into  
bugzilla buys us that is worth the order(s) of magnitude more effort  
that approach would take.


Ojan

On Sat, Jun 6, 2009 at 11:02 AM, Darren VanBuren  
 wrote:
Surprisingly, the bug isn't a duplicate, or if there is a dupe, it  
isn't filed correctly.


But I agree that any code review tool should be integrated with  
bugs.webkit.org, otherwise there would be a huge disorganized mess  
and it wouldn't be any better.


Bugzilla wouldn't be hard to extend for this purpose, just adding a  
field for review status and then making whatever code review tool  
you chose update Bugzilla solves (b).


Some modifications in the tool could also make it attach the patches  
to a bug, and you could also update any field in the bug.


I mean, retiveld seems like a wonderful tool, it seems like  
something that would extend Bugzilla quite nicely. Pushing data to  
Bugzilla can simply be done with XML-RPC according to this page on  
bugzilla.org: http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html 
 and there's plenty of XML libraries for Python you could use to  
work over XML-RPC.


Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 6:21 PM, David Kilzer wrote:

I think this is a great direction to move in, but (IMO) any such  
tool should be tightly integrated with bugs.webkit.org so that (a)  
you don't have to post the same patch more than once, (b) the  
review status of the patch is visible in bugs.webkit.org without  
clicking on a link and (c) it's easy to switch between reviewing  
the patch and updating the bug itself.


I just filed a Bugzilla bug about adding such a feature to Bugzilla  
itself (although I wouldn't be surprised if it's a dupe):


Bugzilla needs better patch review process with annotations and  
versioned patches

<https://bugzilla.mozilla.org/show_bug.cgi?id=496670>

Dave


From: Jeremy Orlow 
To: Ojan Vafai 
Cc: WebKit Development 
Sent: Friday, June 5, 2009 5:08:47 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld

For what it's worth, I definitely think a tool like reitveld would  
help the code review process.  Inline comments and more context  
than the couple lines the diff provides are really, really helpful.


On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:
Sorry in advance for the long email. I'm trying to be thorough.

There's been a lot of discussion on #webkit about possibly using a  
code review tool like reitveld for webkit reviews. There's been  
various suggestions and a few misunderstandings, so it seems worth  
having a more formal discussion about this with the larger WebKit  
community.


The things I'd like to assess here are:
1. Pros/Cons of using a system like reitveld. I listed some below.  
Please add any that I missed.
2. Whether the WebKit community is interested in pursuing something  
like this.

3. If there is interest, what is the best way to move forward.

WHAT IS REITVELD
It's a code review tool. Reitveld doesn't allow you to do anything  
that is impossible with the current review process, however, it  
makes the review process more efficient and less error-prone. As  
such, it makes it easier and less time-consuming to do good,  
thorough code reviews.


The basic gist of reitveld is that it allows you to put comments  
inline and send them all in one chunk. Then it lets the reviewee  
easil

Re: [webkit-dev] to reitveld or not to reitveld

[Ojan Vafai]

This is what I meant by "light-weight" integration. All the review
information would be reflected in the bugzilla bug. You would never be
required to use reitveld for anything.

We would be able to:
1. Add a link to bugzilla that would take you to the reitveld code review
and upload the patch to reitveld if it hasn't been uploaded already.
2. Have all comments published in reitveld be posted to the bug.
3. Have checkboxes in reitveld for r+, r- that would update bugzilla.
4. I think we can even have comments made directly to bugzilla be reflected
in reitveld by having a bot that monitors bugzilla update emails.

A review tool like reitveld is quite a bit of work. Adding similar
functionality to bugzilla itself is a non-trivial amount of work. I don't
see what integrating this functionality any more tightly into bugzilla buys
us that is worth the order(s) of magnitude more effort that approach would
take.

Ojan

On Sat, Jun 6, 2009 at 11:02 AM, Darren VanBuren wrote:

> Surprisingly, the bug isn't a duplicate, or if there is a dupe, it isn't
> filed correctly.
> But I agree that any code review tool should be integrated with
> bugs.webkit.org, otherwise there would be a huge disorganized mess and it
> wouldn't be any better.
>
> Bugzilla wouldn't be hard to extend for this purpose, just adding a field
> for review status and then making whatever code review tool you chose update
> Bugzilla solves (b).
>
> Some modifications in the tool could also make it attach the patches to a
> bug, and you could also update any field in the bug.
>
> I mean, retiveld seems like a wonderful tool, it seems like something that
> would extend Bugzilla quite nicely. Pushing data to Bugzilla can simply be
> done with XML-RPC according to this page on bugzilla.org:
> http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.htmland
>  there's plenty of XML libraries for Python you could use to work over
> XML-RPC.
>
>   Darren VanBuren
> onekop...@gmail.com
> 
> http://oks.tumblr.com/
>
> On Jun 5, 2009, at 6:21 PM, David Kilzer wrote:
>
> I think this is a great direction to move in, but (IMO) any such tool
> should be tightly integrated with bugs.webkit.org so that (a) you don't
> have to post the same patch more than once, (b) the review status of the
> patch is visible in bugs.webkit.org without clicking on a link and (c)
> it's easy to switch between reviewing the patch and updating the bug itself.
>
> I just filed a Bugzilla bug about adding such a feature to Bugzilla itself
> (although I wouldn't be surprised if it's a dupe):
>
> Bugzilla needs better patch review process with annotations and versioned
> patches
> <https://bugzilla.mozilla.org/show_bug.cgi?id=496670>
>
> Dave
>
>
> --
> *From:* Jeremy Orlow 
> *To:* Ojan Vafai 
> *Cc:* WebKit Development 
> *Sent:* Friday, June 5, 2009 5:08:47 PM
> *Subject:* Re: [webkit-dev] to reitveld or not to reitveld
>
> For what it's worth, I definitely think a tool like reitveld would help the
> code review process.  Inline comments and more context than the couple lines
> the diff provides are really, really helpful.
>
> On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:
>
>> Sorry in advance for the long email. I'm trying to be thorough.
>>
>> There's been a lot of discussion on #webkit about possibly using a code
>> review tool like reitveld for webkit reviews. There's been various
>> suggestions and a few misunderstandings, so it seems worth having a more
>> formal discussion about this with the larger WebKit community.
>> The things I'd like to assess here are:
>> 1. Pros/Cons of using a system like reitveld. I listed some below. Please
>> add any that I missed.
>>
>> 2. Whether the WebKit community is interested in pursuing something like 
>> this.
>> 3. If there is interest, what is the best way to move forward.
>>
>> WHAT IS REITVELD
>> It's a code review tool. Reitveld doesn't allow you to do anything that is
>> impossible with the current review process, however, it makes the review
>> process more efficient and less error-prone. As such, it makes it easier and
>> less time-consuming to do good, thorough code reviews.
>>
>> The basic gist of reitveld is that it allows you to put comments inline
>> and send them all in one chunk. Then it lets the reviewee easily respond to
>> each comment individually and send all the responses in one chunk.
>>
>> EXAMPLE CHROMIUM PATCH
>> http://codereview.chromium.org/119103
>>
>> Note that you can view the patch in each version that

Re: [webkit-dev] to reitveld or not to reitveld

[Darren VanBuren]

Surprisingly, the bug isn't a duplicate, or if there is a dupe, it  
isn't filed correctly.


But I agree that any code review tool should be integrated with  
bugs.webkit.org, otherwise there would be a huge disorganized mess and  
it wouldn't be any better.


Bugzilla wouldn't be hard to extend for this purpose, just adding a  
field for review status and then making whatever code review tool you  
chose update Bugzilla solves (b).


Some modifications in the tool could also make it attach the patches  
to a bug, and you could also update any field in the bug.


I mean, retiveld seems like a wonderful tool, it seems like something  
that would extend Bugzilla quite nicely. Pushing data to Bugzilla can  
simply be done with XML-RPC according to this page on bugzilla.org: http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/Server/XMLRPC.html 
 and there's plenty of XML libraries for Python you could use to work  
over XML-RPC.


Darren VanBuren
onekop...@gmail.com

http://oks.tumblr.com/

On Jun 5, 2009, at 6:21 PM, David Kilzer wrote:

I think this is a great direction to move in, but (IMO) any such  
tool should be tightly integrated with bugs.webkit.org so that (a)  
you don't have to post the same patch more than once, (b) the review  
status of the patch is visible in bugs.webkit.org without clicking  
on a link and (c) it's easy to switch between reviewing the patch  
and updating the bug itself.


I just filed a Bugzilla bug about adding such a feature to Bugzilla  
itself (although I wouldn't be surprised if it's a dupe):


Bugzilla needs better patch review process with annotations and  
versioned patches

<https://bugzilla.mozilla.org/show_bug.cgi?id=496670>

Dave


From: Jeremy Orlow 
To: Ojan Vafai 
Cc: WebKit Development 
Sent: Friday, June 5, 2009 5:08:47 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld

For what it's worth, I definitely think a tool like reitveld would  
help the code review process.  Inline comments and more context than  
the couple lines the diff provides are really, really helpful.


On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:
Sorry in advance for the long email. I'm trying to be thorough.

There's been a lot of discussion on #webkit about possibly using a  
code review tool like reitveld for webkit reviews. There's been  
various suggestions and a few misunderstandings, so it seems worth  
having a more formal discussion about this with the larger WebKit  
community.


The things I'd like to assess here are:
1. Pros/Cons of using a system like reitveld. I listed some below.  
Please add any that I missed.
2. Whether the WebKit community is interested in pursuing something  
like this.

3. If there is interest, what is the best way to move forward.

WHAT IS REITVELD
It's a code review tool. Reitveld doesn't allow you to do anything  
that is impossible with the current review process, however, it  
makes the review process more efficient and less error-prone. As  
such, it makes it easier and less time-consuming to do good,  
thorough code reviews.


The basic gist of reitveld is that it allows you to put comments  
inline and send them all in one chunk. Then it lets the reviewee  
easily respond to each comment individually and send all the  
responses in one chunk.


EXAMPLE CHROMIUM PATCH
http://codereview.chromium.org/119103

Note that you can view the patch in each version that was uploaded  
and that you can diff between versions. Also, if a comment was made  
in the version you were looking at, then you can see all the  
comments/responses.


To see this nicely, under "Delta from patch set" in patch set 3,  
click on 2. That is where most of the comments in this review were  
made. For example, http://codereview.chromium.org/119103/diff2/14:27/29 
. You can see all the comments and responses along with the diff in  
the patch to see that the reviewer comments were implemented as  
intended.


Keyboard shortcuts to try out:
-n/p to switch between diff chunks
-shift n/p to switch between comments
-j/k to go to the next/previous file

*Please don't actually click the "Publish all my drafts" button on  
the publish page as you'll be modifying a real code review.*


Other things to try
-try the side-by-side diff and the unified diff views
-adding comments (double click)
-replying to comments
-go to the publish page (click the publish link or type "m")

Also note that the "Committed" URL is automatically added when the  
patch is committed and the reitveld issue is marked closed. So there  
isn't extra overhead in maintaining list of outstanding code reviews.


HOW TO TRY IT OUT
Here's the process for trying out reitveld with a webkit patch. The  
current workflow is a bit janky, but some scripting and some simple  
reitveld fixes would make this a lot more natural and automa

Re: [webkit-dev] to reitveld or not to reitveld

[David Kilzer]

I think this is a great direction to move in, but (IMO) any such tool should be 
tightly integrated with bugs.webkit.org so that (a) you don't have to post the 
same patch more than once, (b) the review status of the patch is visible in 
bugs.webkit.org without clicking on a link and (c) it's easy to switch between 
reviewing the patch and updating the bug itself.

I just filed a Bugzilla bug about adding such a feature to Bugzilla itself 
(although I wouldn't be surprised if it's a dupe):

Bugzilla needs better patch review process with annotations and versioned 
patches
<https://bugzilla.mozilla.org/show_bug.cgi?id=496670>

Dave





From: Jeremy Orlow 
To: Ojan Vafai 
Cc: WebKit Development 
Sent: Friday, June 5, 2009 5:08:47 PM
Subject: Re: [webkit-dev] to reitveld or not to reitveld

For what it's worth, I definitely think a tool like reitveld would help the 
code review process.  Inline comments and more context than the couple lines 
the diff provides are really, really helpful.


On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:

Sorry in advance for the long email. I'm trying to be thorough.
There's been a lot of discussion on #webkit about possibly using a code review 
tool like reitveld for webkit reviews. There's been various suggestions and a 
few misunderstandings, so it seems worth having a more formal discussion about 
this with the larger WebKit community.

The things I'd like to assess here are:
1. Pros/Cons of using a system like reitveld. I listed some below. Please add 
any that I missed.
2. Whether the WebKit community is interested in pursuing something like this.
3. If there is interest, what is the best way to move forward.


WHAT IS REITVELD
It's a code review tool. Reitveld doesn't allow you to do anything that is 
impossible with the current review process, however, it makes the review 
process more efficient and less error-prone. As such, it makes it easier and 
less time-consuming to do good, thorough code reviews.

The basic gist of reitveld is that it allows you to put comments inline and 
send them all in one chunk. Then it lets the reviewee easily respond to each 
comment individually and send all the responses in one chunk.

EXAMPLE CHROMIUM PATCH
http://codereview.chromium.org/119103

Note that you can view the patch in each version that was uploaded and that you 
can diff between versions. Also, if a comment was made in the version you were 
looking at, then you can see all the comments/responses.

To see this nicely, under "Delta from patch set" in patch set 3, click on 2. 
That is where most of the comments in this review were made. For example, 
http://codereview.chromium.org/119103/diff2/14:27/29. You can see all the 
comments and responses along with the diff in the patch to see that the 
reviewer comments were implemented as intended.

Keyboard shortcuts to try out:
-n/p to switch between diff chunks
-shift n/p to switch between comments
-j/k to go to the next/previous file

*Please don't actually click the "Publish all my drafts" button on the publish 
page as you'll be modifying a real code review.*

Other things to try
-try the side-by-side diff and the unified diff views
-adding comments (double click)
-replying to comments
-go to the publish page (click the publish link or type "m") 

Also note that the "Committed" URL is automatically added when the patch is 
committed and the reitveld issue is marked closed. So there isn't extra 
overhead in maintaining list of outstanding code reviews.

HOW TO TRY IT OUT
Here's the process for trying out reitveld with a webkit patch. The current 
workflow is a bit janky, but some scripting and some simple reitveld fixes 
would make this a lot more natural and automated (e.g. chromium uses 
commandline "gcl upload" to put up a new patch).

1. Find a non-git patch
2. Go to http://codereview.chromium.org/new
3. Login with a Google account (e.g. any gmail or Google search account)
4. On that page, type in a subject and paste in the URL to the patch in the URL 
field.
5. Click "Create Issue"

There's a couple apparent bugs that are easily fixable:
1. The ChangeLog files don't get downloaded correct, so the diffs don't work. 
This is an AppEngine problem that Chromium works around with the gcl upload 
script.
2. With an old patch there are often diff chunk mistmatches, which breaks the 
side-by-side diff view (you can use the unified diff in those cases). 

PROS
For the reviewer:
-easier to write thorough review comments since adding comments is so 
light-weight
-easier to make sure that all review comments actually got implemented

For the reviewee:
-easier to see which line the reviewer's comment addresses
-easier to make sure you've completed all the reviewer's comments (you can mark 
them as "done" in reitvel

Re: [webkit-dev] to reitveld or not to reitveld

[Jeremy Orlow]

For what it's worth, I definitely think a tool like reitveld would help the
code review process.  Inline comments and more context than the couple lines
the diff provides are really, really helpful.

On Fri, Jun 5, 2009 at 9:25 AM, Ojan Vafai  wrote:

> Sorry in advance for the long email. I'm trying to be thorough.
>
> There's been a lot of discussion on #webkit about possibly using a code
> review tool like reitveld for webkit reviews. There's been various
> suggestions and a few misunderstandings, so it seems worth having a more
> formal discussion about this with the larger WebKit community.
> The things I'd like to assess here are:
> 1. Pros/Cons of using a system like reitveld. I listed some below. Please
> add any that I missed.
>
> 2. Whether the WebKit community is interested in pursuing something like this.
> 3. If there is interest, what is the best way to move forward.
>
> WHAT IS REITVELD
> It's a code review tool. Reitveld doesn't allow you to do anything that is
> impossible with the current review process, however, it makes the review
> process more efficient and less error-prone. As such, it makes it easier and
> less time-consuming to do good, thorough code reviews.
>
> The basic gist of reitveld is that it allows you to put comments inline and
> send them all in one chunk. Then it lets the reviewee easily respond to each
> comment individually and send all the responses in one chunk.
>
> EXAMPLE CHROMIUM PATCH
> http://codereview.chromium.org/119103
>
> Note that you can view the patch in each version that was uploaded and that
> you can diff between versions. Also, if a comment was made in the version
> you were looking at, then you can see all the comments/responses.
>
> To see this nicely, under "Delta from patch set" in patch set 3, click on
> 2. That is where most of the comments in this review were made. For
> example, http://codereview.chromium.org/119103/diff2/14:27/29. You can see
> all the comments and responses along with the diff in the patch to see that
> the reviewer comments were implemented as intended.
>
> Keyboard shortcuts to try out:
> -n/p to switch between diff chunks
> -shift n/p to switch between comments
> -j/k to go to the next/previous file
>
> *Please don't actually click the "Publish all my drafts" button on the
> publish page as you'll be modifying a real code review.*
>
> Other things to try
> -try the side-by-side diff and the unified diff views
> -adding comments (double click)
> -replying to comments
> -go to the publish page (click the publish link or type "m")
>
> Also note that the "Committed" URL is automatically added when the patch is
> committed and the reitveld issue is marked closed. So there isn't extra
> overhead in maintaining list of outstanding code reviews.
>
> HOW TO TRY IT OUT
> Here's the process for trying out reitveld with a webkit patch. The current
> workflow is a bit janky, but some scripting and some simple reitveld fixes
> would make this a lot more natural and automated (e.g. chromium uses
> commandline "gcl upload" to put up a new patch).
>
> 1. Find a non-git patch
> 2. Go to http://codereview.chromium.org/new
> 3. Login with a Google account (e.g. any gmail or Google search account)
> 4. On that page, type in a subject and paste in the URL to the patch in the
> URL field.
> 5. Click "Create Issue"
>
> There's a couple apparent bugs that are easily fixable:
> 1. The ChangeLog files don't get downloaded correct, so the diffs don't
> work. This is an AppEngine problem that Chromium works around with the gcl
> upload script.
> 2. With an old patch there are often diff chunk mistmatches, which breaks
> the side-by-side diff view (you can use the unified diff in those cases).
>
> PROS
> For the reviewer:
> -easier to write thorough review comments since adding comments is so
> light-weight
> -easier to make sure that all review comments actually got implemented
>
> For the reviewee:
> -easier to see which line the reviewer's comment addresses
> -easier to make sure you've completed all the reviewer's comments (you can
> mark them as "done" in reitveld as you go)
>
> For everyone:
> -efficient keyboard navigation (e.g. j/k to navigation between diff chunks
> and n/p to navigate between files
> -easier to follow the progression of a code review and see what changed
> over the course of the review
> -shows image files, so you can see before/after before commit
>
> CONS (most of these are easy to fix/improve)
> -There's no pretty printed view of all the files in the patch at once that
> lets you insert comments
> -The UI is a bit cluttered
> -It takes using it for a couple patches before you're totally comfortable
> with it
> -Currently doesn't work with diffs generated by git
> -Reitveld's current implementation requires running on AppEngine
> -A couple issues with reitveld on appengine that Chromium uses a script to
> workaround (line-endings differences and large files like ChangeLogs don't
> upload correclty).
>
> PATH FORWARD
> As

[webkit-dev] to reitveld or not to reitveld

[Ojan Vafai]

Sorry in advance for the long email. I'm trying to be thorough.

There's been a lot of discussion on #webkit about possibly using a code
review tool like reitveld for webkit reviews. There's been various
suggestions and a few misunderstandings, so it seems worth having a more
formal discussion about this with the larger WebKit community.
The things I'd like to assess here are:
1. Pros/Cons of using a system like reitveld. I listed some below. Please
add any that I missed.
2. Whether the WebKit community is interested in pursuing something like this.
3. If there is interest, what is the best way to move forward.

WHAT IS REITVELD
It's a code review tool. Reitveld doesn't allow you to do anything that is
impossible with the current review process, however, it makes the review
process more efficient and less error-prone. As such, it makes it easier and
less time-consuming to do good, thorough code reviews.

The basic gist of reitveld is that it allows you to put comments inline and
send them all in one chunk. Then it lets the reviewee easily respond to each
comment individually and send all the responses in one chunk.

EXAMPLE CHROMIUM PATCH
http://codereview.chromium.org/119103

Note that you can view the patch in each version that was uploaded and that
you can diff between versions. Also, if a comment was made in the version
you were looking at, then you can see all the comments/responses.

To see this nicely, under "Delta from patch set" in patch set 3, click on 2.
That is where most of the comments in this review were made. For example,
http://codereview.chromium.org/119103/diff2/14:27/29. You can see all the
comments and responses along with the diff in the patch to see that the
reviewer comments were implemented as intended.

Keyboard shortcuts to try out:
-n/p to switch between diff chunks
-shift n/p to switch between comments
-j/k to go to the next/previous file

*Please don't actually click the "Publish all my drafts" button on the
publish page as you'll be modifying a real code review.*

Other things to try
-try the side-by-side diff and the unified diff views
-adding comments (double click)
-replying to comments
-go to the publish page (click the publish link or type "m")

Also note that the "Committed" URL is automatically added when the patch is
committed and the reitveld issue is marked closed. So there isn't extra
overhead in maintaining list of outstanding code reviews.

HOW TO TRY IT OUT
Here's the process for trying out reitveld with a webkit patch. The current
workflow is a bit janky, but some scripting and some simple reitveld fixes
would make this a lot more natural and automated (e.g. chromium uses
commandline "gcl upload" to put up a new patch).

1. Find a non-git patch
2. Go to http://codereview.chromium.org/new
3. Login with a Google account (e.g. any gmail or Google search account)
4. On that page, type in a subject and paste in the URL to the patch in the
URL field.
5. Click "Create Issue"

There's a couple apparent bugs that are easily fixable:
1. The ChangeLog files don't get downloaded correct, so the diffs don't
work. This is an AppEngine problem that Chromium works around with the gcl
upload script.
2. With an old patch there are often diff chunk mistmatches, which breaks
the side-by-side diff view (you can use the unified diff in those cases).

PROS
For the reviewer:
-easier to write thorough review comments since adding comments is so
light-weight
-easier to make sure that all review comments actually got implemented

For the reviewee:
-easier to see which line the reviewer's comment addresses
-easier to make sure you've completed all the reviewer's comments (you can
mark them as "done" in reitveld as you go)

For everyone:
-efficient keyboard navigation (e.g. j/k to navigation between diff chunks
and n/p to navigate between files
-easier to follow the progression of a code review and see what changed over
the course of the review
-shows image files, so you can see before/after before commit

CONS (most of these are easy to fix/improve)
-There's no pretty printed view of all the files in the patch at once that
lets you insert comments
-The UI is a bit cluttered
-It takes using it for a couple patches before you're totally comfortable
with it
-Currently doesn't work with diffs generated by git
-Reitveld's current implementation requires running on AppEngine
-A couple issues with reitveld on appengine that Chromium uses a script to
workaround (line-endings differences and large files like ChangeLogs don't
upload correclty).

PATH FORWARD
As far as reitveld versus another code review tool, I don't have strong
opinions. I hear http://www.review-board.org/ is good, but I haven't used
it. One advantage of using reitveld is that a lot of the work on reitveld
was done by Chromium team members and so modifying it to meet WebKit needs
(e.g. running an instance that isn't tied to Chromium, changing the UI,
etc.) should be relatively painless.

I think the transition to using a new tool