You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Evan Gilbert <ui...@google.com> on 2008/09/12 02:59:28 UTC

Code review tools

Wanted to revive the thread on using code review tools with Shindig. There
is a really good tool up at http://codereview.appspot.com, and I'd like to
discuss whether it is OK to use for reviews.
- Sample diff: http://codereview.appspot.com/5245/diff/1/2
- Full review: http://codereview.appspot.com/5245

Here's my quick list of pluses and minuses...

Good:
- Ability to email cc on reviews, so that the official record is still on
shindig-dev@
- Threaded comment support, very clean inline display of diffs
- Open source, so we can run our own instance if we need to customize
- But hosted for now, so we can easily try it out
- Doesn't require a login to view patches and comments

Bad:
- Hosted on appspot, which requires a Google login to comment. We can change
this if we run our own hosted version.

How do people feel about trying this out on an experimental basis?

Evan

P.S. Does anyone know what the URL format is to access for revisions of
files in svn? It looks like the diffs only work against the most recent
version in SVN, as it is using
http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
?*rev=690667* as the URL format, which doesn't actually get the bolded
revision.

Re: Code review tools

Posted by Santiago Gala <sa...@gmail.com>.
On Tue, Sep 16, 2008 at 6:10 PM, Evan Gilbert <ui...@google.com> wrote:
> The code uses the persistence layer and users from Google app engine
> currently, but these look easy to swap out if we want - there's only about
> 150K of code in the whole project, and a lot of this is for smart diffing,
> etc. See http://code.google.com/p/rietveld/source/browse/.
>

I  think people is working on the django model layer, so that it will
be able to work with Google AppEngine and also with local DB, but
things are not really there yet.

> Possibly a good next step is to try it out for a few code reviews and see
> how it works with our dev processes? Since it sends email with the comments,
> usage would be completely optional.
>
> Evan
>
> P.S. Still need to figure out the URL to get to a specific version of a file
> in SVN (for programmatic access, not just for browsing) - please email me if
> you have this.
>

There is a dirty,  undocumented trick involving some magics !vc...
etc, but I think using the client and the libs is the reccomended way
to go.

Regards
Santiago

> On Fri, Sep 12, 2008 at 9:53 AM, Santiago Gala <sa...@gmail.com>wrote:
>
>> Answer via cellular.
>>
>> Is'ot the soft designed to work only with google app engine? Not
>> tracking so closely...
>>
>> 2008/9/12, Evan Gilbert <ui...@google.com>:
>> > On Thu, Sep 11, 2008 at 6:25 PM, John Hjelmstad <fa...@google.com>
>> wrote:
>> >
>> >> On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <ui...@google.com>
>> wrote:
>> >>
>> >> > Wanted to revive the thread on using code review tools with Shindig.
>> >> There
>> >> > is a really good tool up at http://codereview.appspot.com, and I'd
>> like
>> >> to
>> >> > discuss whether it is OK to use for reviews.
>> >> > - Sample diff: http://codereview.appspot.com/5245/diff/1/2
>> >> > - Full review: http://codereview.appspot.com/5245
>> >> >
>> >> > Here's my quick list of pluses and minuses...
>> >> >
>> >> > Good:
>> >> > - Ability to email cc on reviews, so that the official record is still
>> >> > on
>> >> > shindig-dev@
>> >> > - Threaded comment support, very clean inline display of diffs
>> >> > - Open source, so we can run our own instance if we need to customize
>> >> > - But hosted for now, so we can easily try it out
>> >> > - Doesn't require a login to view patches and comments
>> >>
>> >>
>> >> >
>> >> > Bad:
>> >> > - Hosted on appspot, which requires a Google login to comment. We can
>> >> > change
>> >> > this if we run our own hosted version.
>> >>
>> >>
>> >> How difficult would it be to maintain reasonably current code, were we
>> to
>> >> switch out a different set of IDs? Do you have any sense for how
>> >> extensible
>> >> the tool is, particularly for integration with JIRA, shindig-commit@,
>> et
>> >> al?
>> >
>> >
>> > As long as we have python developers, should be easy. I imagine that it
>> > would be a good enhancement to the core code to support all of these
>> > features via config.
>> >
>> > If not, the code base seems to be about 150K, so I don't think we'll have
>> a
>> > problem keeping it up to date.
>> >
>> > http://code.google.com/p/rietveld/source/browse/#svn/trunk/codereview
>> >
>> >
>> >>
>> >>
>> >> In general I'm all for it, particularly since it already works (per your
>> >> example).. at least for patches to start. It's far better than reading
>> >> manually through diffs in email.
>> >>
>> >> --John
>> >>
>> >>
>> >> >
>> >> >
>> >> > How do people feel about trying this out on an experimental basis?
>> >> >
>> >> > Evan
>> >> >
>> >> > P.S. Does anyone know what the URL format is to access for revisions
>> of
>> >> > files in svn? It looks like the diffs only work against the most
>> recent
>> >> > version in SVN, as it is using
>> >> >
>> >> >
>> >>
>> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> >> > ?*rev=690667*<
>> >>
>> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java?*rev=690667*
>> >as
>> >> the URL format, which doesn't actually get the bolded
>> >> > revision.
>> >> >
>> >>
>> >
>>
>

Re: Code review tools

Posted by Evan Gilbert <ui...@google.com>.
The code uses the persistence layer and users from Google app engine
currently, but these look easy to swap out if we want - there's only about
150K of code in the whole project, and a lot of this is for smart diffing,
etc. See http://code.google.com/p/rietveld/source/browse/.

Possibly a good next step is to try it out for a few code reviews and see
how it works with our dev processes? Since it sends email with the comments,
usage would be completely optional.

Evan

P.S. Still need to figure out the URL to get to a specific version of a file
in SVN (for programmatic access, not just for browsing) - please email me if
you have this.

On Fri, Sep 12, 2008 at 9:53 AM, Santiago Gala <sa...@gmail.com>wrote:

> Answer via cellular.
>
> Is'ot the soft designed to work only with google app engine? Not
> tracking so closely...
>
> 2008/9/12, Evan Gilbert <ui...@google.com>:
> > On Thu, Sep 11, 2008 at 6:25 PM, John Hjelmstad <fa...@google.com>
> wrote:
> >
> >> On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <ui...@google.com>
> wrote:
> >>
> >> > Wanted to revive the thread on using code review tools with Shindig.
> >> There
> >> > is a really good tool up at http://codereview.appspot.com, and I'd
> like
> >> to
> >> > discuss whether it is OK to use for reviews.
> >> > - Sample diff: http://codereview.appspot.com/5245/diff/1/2
> >> > - Full review: http://codereview.appspot.com/5245
> >> >
> >> > Here's my quick list of pluses and minuses...
> >> >
> >> > Good:
> >> > - Ability to email cc on reviews, so that the official record is still
> >> > on
> >> > shindig-dev@
> >> > - Threaded comment support, very clean inline display of diffs
> >> > - Open source, so we can run our own instance if we need to customize
> >> > - But hosted for now, so we can easily try it out
> >> > - Doesn't require a login to view patches and comments
> >>
> >>
> >> >
> >> > Bad:
> >> > - Hosted on appspot, which requires a Google login to comment. We can
> >> > change
> >> > this if we run our own hosted version.
> >>
> >>
> >> How difficult would it be to maintain reasonably current code, were we
> to
> >> switch out a different set of IDs? Do you have any sense for how
> >> extensible
> >> the tool is, particularly for integration with JIRA, shindig-commit@,
> et
> >> al?
> >
> >
> > As long as we have python developers, should be easy. I imagine that it
> > would be a good enhancement to the core code to support all of these
> > features via config.
> >
> > If not, the code base seems to be about 150K, so I don't think we'll have
> a
> > problem keeping it up to date.
> >
> > http://code.google.com/p/rietveld/source/browse/#svn/trunk/codereview
> >
> >
> >>
> >>
> >> In general I'm all for it, particularly since it already works (per your
> >> example).. at least for patches to start. It's far better than reading
> >> manually through diffs in email.
> >>
> >> --John
> >>
> >>
> >> >
> >> >
> >> > How do people feel about trying this out on an experimental basis?
> >> >
> >> > Evan
> >> >
> >> > P.S. Does anyone know what the URL format is to access for revisions
> of
> >> > files in svn? It looks like the diffs only work against the most
> recent
> >> > version in SVN, as it is using
> >> >
> >> >
> >>
> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> >> > ?*rev=690667*<
> >>
> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java?*rev=690667*
> >as
> >> the URL format, which doesn't actually get the bolded
> >> > revision.
> >> >
> >>
> >
>

Re: Code review tools

Posted by Santiago Gala <sa...@gmail.com>.
Answer via cellular.

Is'ot the soft designed to work only with google app engine? Not
tracking so closely...

2008/9/12, Evan Gilbert <ui...@google.com>:
> On Thu, Sep 11, 2008 at 6:25 PM, John Hjelmstad <fa...@google.com> wrote:
>
>> On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <ui...@google.com> wrote:
>>
>> > Wanted to revive the thread on using code review tools with Shindig.
>> There
>> > is a really good tool up at http://codereview.appspot.com, and I'd like
>> to
>> > discuss whether it is OK to use for reviews.
>> > - Sample diff: http://codereview.appspot.com/5245/diff/1/2
>> > - Full review: http://codereview.appspot.com/5245
>> >
>> > Here's my quick list of pluses and minuses...
>> >
>> > Good:
>> > - Ability to email cc on reviews, so that the official record is still
>> > on
>> > shindig-dev@
>> > - Threaded comment support, very clean inline display of diffs
>> > - Open source, so we can run our own instance if we need to customize
>> > - But hosted for now, so we can easily try it out
>> > - Doesn't require a login to view patches and comments
>>
>>
>> >
>> > Bad:
>> > - Hosted on appspot, which requires a Google login to comment. We can
>> > change
>> > this if we run our own hosted version.
>>
>>
>> How difficult would it be to maintain reasonably current code, were we to
>> switch out a different set of IDs? Do you have any sense for how
>> extensible
>> the tool is, particularly for integration with JIRA, shindig-commit@, et
>> al?
>
>
> As long as we have python developers, should be easy. I imagine that it
> would be a good enhancement to the core code to support all of these
> features via config.
>
> If not, the code base seems to be about 150K, so I don't think we'll have a
> problem keeping it up to date.
>
> http://code.google.com/p/rietveld/source/browse/#svn/trunk/codereview
>
>
>>
>>
>> In general I'm all for it, particularly since it already works (per your
>> example).. at least for patches to start. It's far better than reading
>> manually through diffs in email.
>>
>> --John
>>
>>
>> >
>> >
>> > How do people feel about trying this out on an experimental basis?
>> >
>> > Evan
>> >
>> > P.S. Does anyone know what the URL format is to access for revisions of
>> > files in svn? It looks like the diffs only work against the most recent
>> > version in SVN, as it is using
>> >
>> >
>> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
>> > ?*rev=690667*<
>> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java?*rev=690667*>as
>> the URL format, which doesn't actually get the bolded
>> > revision.
>> >
>>
>

Re: Code review tools

Posted by Evan Gilbert <ui...@google.com>.
On Thu, Sep 11, 2008 at 6:25 PM, John Hjelmstad <fa...@google.com> wrote:

> On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <ui...@google.com> wrote:
>
> > Wanted to revive the thread on using code review tools with Shindig.
> There
> > is a really good tool up at http://codereview.appspot.com, and I'd like
> to
> > discuss whether it is OK to use for reviews.
> > - Sample diff: http://codereview.appspot.com/5245/diff/1/2
> > - Full review: http://codereview.appspot.com/5245
> >
> > Here's my quick list of pluses and minuses...
> >
> > Good:
> > - Ability to email cc on reviews, so that the official record is still on
> > shindig-dev@
> > - Threaded comment support, very clean inline display of diffs
> > - Open source, so we can run our own instance if we need to customize
> > - But hosted for now, so we can easily try it out
> > - Doesn't require a login to view patches and comments
>
>
> >
> > Bad:
> > - Hosted on appspot, which requires a Google login to comment. We can
> > change
> > this if we run our own hosted version.
>
>
> How difficult would it be to maintain reasonably current code, were we to
> switch out a different set of IDs? Do you have any sense for how extensible
> the tool is, particularly for integration with JIRA, shindig-commit@, et
> al?


As long as we have python developers, should be easy. I imagine that it
would be a good enhancement to the core code to support all of these
features via config.

If not, the code base seems to be about 150K, so I don't think we'll have a
problem keeping it up to date.

http://code.google.com/p/rietveld/source/browse/#svn/trunk/codereview


>
>
> In general I'm all for it, particularly since it already works (per your
> example).. at least for patches to start. It's far better than reading
> manually through diffs in email.
>
> --John
>
>
> >
> >
> > How do people feel about trying this out on an experimental basis?
> >
> > Evan
> >
> > P.S. Does anyone know what the URL format is to access for revisions of
> > files in svn? It looks like the diffs only work against the most recent
> > version in SVN, as it is using
> >
> >
> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> > ?*rev=690667*<
> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java?*rev=690667*>as
> the URL format, which doesn't actually get the bolded
> > revision.
> >
>

Re: Code review tools

Posted by John Hjelmstad <fa...@google.com>.
On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <ui...@google.com> wrote:

> Wanted to revive the thread on using code review tools with Shindig. There
> is a really good tool up at http://codereview.appspot.com, and I'd like to
> discuss whether it is OK to use for reviews.
> - Sample diff: http://codereview.appspot.com/5245/diff/1/2
> - Full review: http://codereview.appspot.com/5245
>
> Here's my quick list of pluses and minuses...
>
> Good:
> - Ability to email cc on reviews, so that the official record is still on
> shindig-dev@
> - Threaded comment support, very clean inline display of diffs
> - Open source, so we can run our own instance if we need to customize
> - But hosted for now, so we can easily try it out
> - Doesn't require a login to view patches and comments


>
> Bad:
> - Hosted on appspot, which requires a Google login to comment. We can
> change
> this if we run our own hosted version.


How difficult would it be to maintain reasonably current code, were we to
switch out a different set of IDs? Do you have any sense for how extensible
the tool is, particularly for integration with JIRA, shindig-commit@, et al?

In general I'm all for it, particularly since it already works (per your
example).. at least for patches to start. It's far better than reading
manually through diffs in email.

--John


>
>
> How do people feel about trying this out on an experimental basis?
>
> Evan
>
> P.S. Does anyone know what the URL format is to access for revisions of
> files in svn? It looks like the diffs only work against the most recent
> version in SVN, as it is using
>
> http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java
> ?*rev=690667*<http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java?*rev=690667*>as the URL format, which doesn't actually get the bolded
> revision.
>

Re: Code review tools

Posted by Tim Moore <tm...@atlassian.com>.
On Sep 16, 2008, at 9:16 AM, Evan Gilbert wrote:

> Hi Tim - to help evaluate it would be good to have a sample code  
> review
> patch posted. I'm happy with any tool that code a great job of  
> making code
> reviews easy.
>
> Also, possibly someone from the Apache side can comment on using  
> commercial
> software vs. open source for tools (although we're using JIRA  
> already, so
> this seems possibly moot).


Evan,

We have a bunch of Apache projects hosted already at http://fisheye6.atlassian.com/

The way the software works is that FishEye indexes your svn repository  
and provides a web UI, then Crucible extends that to offer code  
reviews. You can also upload patch files if you want to do a pre- 
commit review. Let me know if you need a hand with anything.

Feel free to sign up for an account at fisheye6 and create reviews in  
the DEMO project there, either for one of the projects that is on  
there already or for a patch file from Shindig.

If you decide its something you'd like to use, we can either host a  
project right there on fisheye6 or get you an open source license to  
run your own copy. I think we'd just need to get permission from  
Apache infrastructure if you want us to index the svn repository.

Cheers,
-- 
Tim Moore
Plugin Developer, Atlassian




Re: Code review tools

Posted by Evan Gilbert <ui...@google.com>.
Hi Tim - to help evaluate it would be good to have a sample code review
patch posted. I'm happy with any tool that code a great job of making code
reviews easy.

Also, possibly someone from the Apache side can comment on using commercial
software vs. open source for tools (although we're using JIRA already, so
this seems possibly moot).

Evan

On Sat, Sep 13, 2008 at 10:43 AM, Tim Moore <tm...@atlassian.com> wrote:

> I apologize if this is inappropriate, but I work as a developer for
> Atlassian, the company that makes JIRA. I'm on the list because I'm
> currently working on a project to use gadgets in the JIRA dashboard. We also
> make a code review tool called Crucible that you might want to look at, too.
> http://www.atlassian.com/software/crucible/
>
> It is a commercial product, but like all our software, we offer free
> licenses to open source projects: https://www.atlassian.com/opensource/
>
> You could host it yourself, or use it as part of the JIRA Studio hosted
> suite: http://www.atlassian.com/hosted/studio/
>
> It does support integration with JIRA, naturally, although if you use the
> hosted version, it's set up to integrate with the hosted JIRA instance, so
> if you wanted to do that I would recommend migrating your current JIRA
> project into JIRA Studio.
>
> If this is something that interests the group, feel free to ask me
> questions either on or off-list. If it's not something that interests you,
> then I'll shut up about it :-) Either way, I'm glad to see the interest in
> adopting a code review tool, since I've personally found them to make the
> process of reviewing code less of a headache and often pretty fun!
>
> Cheers,
> --
> Tim Moore
> Atlassian Plugin Developer
>
>
>
>

Re: Code review tools

Posted by Tim Moore <tm...@atlassian.com>.
I apologize if this is inappropriate, but I work as a developer for  
Atlassian, the company that makes JIRA. I'm on the list because I'm  
currently working on a project to use gadgets in the JIRA dashboard.  
We also make a code review tool called Crucible that you might want to  
look at, too. http://www.atlassian.com/software/crucible/

It is a commercial product, but like all our software, we offer free  
licenses to open source projects: https://www.atlassian.com/opensource/

You could host it yourself, or use it as part of the JIRA Studio  
hosted suite: http://www.atlassian.com/hosted/studio/

It does support integration with JIRA, naturally, although if you use  
the hosted version, it's set up to integrate with the hosted JIRA  
instance, so if you wanted to do that I would recommend migrating your  
current JIRA project into JIRA Studio.

If this is something that interests the group, feel free to ask me  
questions either on or off-list. If it's not something that interests  
you, then I'll shut up about it :-) Either way, I'm glad to see the  
interest in adopting a code review tool, since I've personally found  
them to make the process of reviewing code less of a headache and  
often pretty fun!

Cheers,
-- 
Tim Moore
Atlassian Plugin Developer