On Thu, Sep 11, 2008 at 6:25 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote:
> On Thu, Sep 11, 2008 at 5:59 PM, Evan Gilbert <[EMAIL PROTECTED]> 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. > > >
