Re: Code review tools

[Sean Mackrory <ma...@gmail.com>]

 Moved to dev for obvious reasons...

+1 to the idea of using ReviewBoard. It would certainly be nicer for more
complicated changes instead of discussing on the JIRA.


On Tue, Nov 27, 2012 at 5:08 PM, Mark Grover <gr...@gmail.com>wrote:

> Hi all,
> When reviewing Bigtop patches, I often find myself downloading the patch,
> applying it to my local repo and using a diff tool on my computer to review
> the patch (and have some context around it). I know for a fact that some of
> the other Apache projects are very good about asking people to post reviews
> on reviewboard (or something similar) when uploading a new patch. That
> makes the process of reviewing the diff and commenting on patches much
> easier.
>
> Would it make sense for us to start leveraging something like
> https://reviews.apache.org/dashboard/ for this?
>
> Thoughts?
>
> Mark
>

Re: Code review tools

[Sean Mackrory <ma...@gmail.com>]

To clarify my +1 was for it being available - I agree with Bruno that it
would only be nice for large patches or changes where small implementation
details are likely to be discussed heavily. Outside of that the current
system is good.


On Wed, Nov 28, 2012 at 12:33 AM, Bruno Mahé <bm...@apache.org> wrote:

> On 11/27/2012 05:20 PM, Sean Mackrory wrote:
>
>>   Moved to dev for obvious reasons...
>>
>> +1 to the idea of using ReviewBoard. It would certainly be nicer for more
>> complicated changes instead of discussing on the JIRA.
>>
>>
>> On Tue, Nov 27, 2012 at 5:08 PM, Mark Grover <grover.markgrover@gmail.com
>> >**wrote:
>>
>>  Hi all,
>>> When reviewing Bigtop patches, I often find myself downloading the patch,
>>> applying it to my local repo and using a diff tool on my computer to
>>> review
>>> the patch (and have some context around it). I know for a fact that some
>>> of
>>> the other Apache projects are very good about asking people to post
>>> reviews
>>> on reviewboard (or something similar) when uploading a new patch. That
>>> makes the process of reviewing the diff and commenting on patches much
>>> easier.
>>>
>>> Would it make sense for us to start leveraging something like
>>> https://reviews.apache.org/**dashboard/<https://reviews.apache.org/dashboard/>for this?
>>>
>>> Thoughts?
>>>
>>> Mark
>>>
>>>
>>
> As long as its usage is voluntary and not forced/requested, I am ok with
> it.
>
> Most patches are small and easy to review just by looking at the diff. So
> forcing to upload a diff would not bring much value. But that should not
> prevent people who like using reviewboard to just post their patch there.
>
> Also, applying the patch locally enables me to use *my* diffing tool *I*
> like and configured and adapted to *my* usage without imposing any
> constraint on others.
> For bigger patches using a review tool would make more sense thanks to the
> fine grained commenting capabilities.
>
> Thanks,
> Bruno
>
>
>

Re: Code review tools

[Bruno Mahé <bm...@apache.org>]

On 11/27/2012 05:20 PM, Sean Mackrory wrote:
>   Moved to dev for obvious reasons...
>
> +1 to the idea of using ReviewBoard. It would certainly be nicer for more
> complicated changes instead of discussing on the JIRA.
>
>
> On Tue, Nov 27, 2012 at 5:08 PM, Mark Grover <gr...@gmail.com>wrote:
>
>> Hi all,
>> When reviewing Bigtop patches, I often find myself downloading the patch,
>> applying it to my local repo and using a diff tool on my computer to review
>> the patch (and have some context around it). I know for a fact that some of
>> the other Apache projects are very good about asking people to post reviews
>> on reviewboard (or something similar) when uploading a new patch. That
>> makes the process of reviewing the diff and commenting on patches much
>> easier.
>>
>> Would it make sense for us to start leveraging something like
>> https://reviews.apache.org/dashboard/ for this?
>>
>> Thoughts?
>>
>> Mark
>>
>

As long as its usage is voluntary and not forced/requested, I am ok with it.

Most patches are small and easy to review just by looking at the diff. 
So forcing to upload a diff would not bring much value. But that should 
not prevent people who like using reviewboard to just post their patch 
there.

Also, applying the patch locally enables me to use *my* diffing tool *I* 
like and configured and adapted to *my* usage without imposing any 
constraint on others.
For bigger patches using a review tool would make more sense thanks to 
the fine grained commenting capabilities.

Thanks,
Bruno

Re: Code review tools

[Konstantin Boudnik <co...@apache.org>]

I guess JIRA discussion has its own merits, but using visual aid won't hurt.
Too bad we can't use github like PR tools though ...

Cos

On Tue, Nov 27, 2012 at 05:20PM, Sean Mackrory wrote:
>  Moved to dev for obvious reasons...
> 
> +1 to the idea of using ReviewBoard. It would certainly be nicer for more
> complicated changes instead of discussing on the JIRA.
> 
> 
> On Tue, Nov 27, 2012 at 5:08 PM, Mark Grover <gr...@gmail.com>wrote:
> 
> > Hi all,
> > When reviewing Bigtop patches, I often find myself downloading the patch,
> > applying it to my local repo and using a diff tool on my computer to review
> > the patch (and have some context around it). I know for a fact that some of
> > the other Apache projects are very good about asking people to post reviews
> > on reviewboard (or something similar) when uploading a new patch. That
> > makes the process of reviewing the diff and commenting on patches much
> > easier.
> >
> > Would it make sense for us to start leveraging something like
> > https://reviews.apache.org/dashboard/ for this?
> >
> > Thoughts?
> >
> > Mark
> >