[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 <mrowe at apple.com> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20090606/d029f8b8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3166 bytes
Desc: not available
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20090606/d029f8b8/attachment.bin>