Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Bigtop >> mail # user >> Code review tools


Copy link to this message
-
Re: Code review tools
Thanks, everyone for your input.

The general consensus seems to be the following:
1. Regardless of whether a review tool is used or not, the patch should
always be uploaded on the JIRA.
2. We don't force people to upload the patch on the review tool. For
patches that are non-trivial, uploaders would be encouraged to upload the
patch on the review tool.

I didn't see any particular objection against review board and it seems
commonly used in other projects so I went ahead and created an INFRA JIRA
for setting up Bigtop as a project on Reviewboard (
https://reviews.apache.org/dashboard/):
https://issues.apache.org/jira/browse/INFRA-5594

Let me know if there is anything I missed.

Thanks again!
Mark

On Wed, Nov 28, 2012 at 1:02 PM, Konstantin Boudnik <[EMAIL PROTECTED]> wrote:

> Sounds like an awful amount of work for patches that usually like 15 lines
> long.
>
> On Thu, Nov 29, 2012 at 01:59AM, Harsh J wrote:
> > FWIW, Hive uses Phabricator for this that helps automate the
> > discussion cross-posting, and also uploads all revisions of patches.
> >
> > There's also a script Hadoop uses to keep the comments cross posted
> > from RB to JIRA, to not lose them in case of RB outage or extinction,
> > but I don't think it does revision patch uploads (or not yet anyway).
> >
> > Personally, I like the RB UI more, but maybe Phabricator would feel
> > easier to setup.
> >
> > On Thu, Nov 29, 2012 at 1:53 AM, Konstantin Boudnik <[EMAIL PROTECTED]>
> wrote:
> > > On Wed, Nov 28, 2012 at 02:22PM, Brock Noland wrote:
> > >>    Flume does this, that is patches must be posted on JIRA as well
> even if
> > >>    they are on RB.
> > >
> > > Yup,  that would make sense.
> > >
> > > Cos
> > >
> > >>    On Wed, Nov 28, 2012 at 2:11 PM, Konstantin Boudnik <
> [EMAIL PROTECTED]>
> > >>    wrote:
> > >>
> > >>      To chime in with Bruno earlier - having a review board doesn't
> mean that
> > >>      patches shouldn't posted on JIRAs as we do right now. Otherwise,
> this
> > >>      would be
> > >>      an enforcement of the tooling, which I oppose big time.
> > >>
> > >>      Cos
> > >>      On Wed, Nov 28, 2012 at 12:02PM, Jonathan Hsieh wrote:
> > >>      >    I'm not that active here, but if you want bigtop setup on
> > >>      >    reviews.apache.org all it just takes filing a apache INFRA
> jira and
> > >>      a
> > >>      >    little patience to get it done.  Once it is up, you post
> patches
> > >>      there.
> > >>      >     I'm from HBase-land and I generally ask for review board
> only on
> > >>      patches
> > >>      >    that are larger than a 1-2 screen-fulls.
> > >>      >
> > >>      >    Jon.
> > >>      >    On Tue, Nov 27, 2012 at 5:08 PM, Mark Grover
> > >>      <[EMAIL PROTECTED]>
> > >>      >    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
> > >>      >
> > >>      >    --
> > >>      >    // Jonathan Hsieh (shay)
> > >>      >    // Software Engineer, Cloudera
> > >>      >    // [EMAIL PROTECTED]
> > >>      >
> > >>
> > >>    --
> > >>    Apache MRUnit - Unit testing MapReduce -
> > >>    http://incubator.apache.org/mrunit/
> >
> >
> >
> > --
> > Harsh J
>