hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Arpit Agarwal <aagar...@hortonworks.com>
Subject Re: Patch review process
Date Tue, 27 Jan 2015 18:48:50 GMT
Filed INFRA-9071 to update reviews.apache.org working with the hadoop git
repo. Let's see if that works out.

Looks like Crucible is no go from infra -
https://issues.apache.org/jira/browse/INFRA-8430.

On Tue, Jan 27, 2015 at 2:50 AM, Steve Loughran <stevel@hortonworks.com>
wrote:

> I'd be +1 on trying reviews.apache.org on a JIRA which
>
>    1. had multiple distributed people working on it
>    2. had some tangible code needing reviewing
>    3. was of limited enough size/duration that we'd see how well it worked
>
> do that, get feedback from the participants and repeat until we're happy
> with a process.
>
> if others can try cruicible at the same time, that would parallelise the
> work.
>
> On 26 January 2015 at 22:41, Chris Nauroth <cnauroth@hortonworks.com>
> wrote:
>
> > reviews.apache.org is backed by Review Board, and I've had a very
> positive
> > experience with that tool on other projects.  HADOOP-9629 is a Hadoop
> patch
> > where we decided to go ahead and use it, and I think it helped.  AFAIK,
> > there is no rule against using it in Hadoop, but it does have the side
> > effect of splitting part of the conversation out of jira.  If Crucible
> can
> > keep all the review notes sync'd with the jira and searchable, then that
> > would be very interesting.
> >
> > Chris Nauroth
> > Hortonworks
> > http://hortonworks.com/
> >
> >
> > On Mon, Jan 26, 2015 at 1:54 PM, Arpit Agarwal <aagarwal@hortonworks.com
> >
> > wrote:
> >
> > > IMO the number one improvement would be a web-based review tool. We
> could
> > > evaluate Atlassian Crucible since it claims to integrate well with
> Jira.
> > I
> > > have not tried https://reviews.apache.org/r/new/.
> > >
> > > Some easy improvements that were also raised by others on the private
> > list:
> > > - Encourage contributors to batch related trivial fixes into a single
> > > patch.
> > > - Require more detailed descriptions with non-trivial patch
> > contributions.
> > > For patches that require knowledge of a specific subsystem a
> > > background+design note would be a good start.
> > > - Eliminate CHANGES.txt. This came up on the dev list not too long ago
> > and
> > > IIRC Allen did a PoC.
> > >
> > > I am not optimistic about Gerrit from past experience. It does help
> gate
> > > checkins and enforce pre-commit checks (good). I did not find it
> > > user-friendly and it will be an additional hurdle for contributors to
> > > understand (bad).
> > >
> > > Andrew, can the community build on your distributed pre-commit work to
> > make
> > > it production ready?
> > >
> > > Regards,
> > > Arpit
> > >
> > >
> > > On Mon, Jan 26, 2015 at 11:55 AM, Andrew Wang <
> andrew.wang@cloudera.com>
> > > wrote:
> > >
> > > > Let's move this over to common-dev@, general@ is normally used for
> > > project
> > > > announcements rather than discussion topics.
> > > >
> > > > I'd like to summarize a few things mentioned on the private@ thread,
> > > > related to streamlining the code submission process.
> > > >
> > > > - Gerrit was brought up again, as it has in the past, as something
> that
> > > > could make the actual process of reviewing and committing a lot
> easier.
> > > > This would be especially helpful for small patches, where the
> mechanics
> > > of
> > > > committing can take longer than reviewing the patch.
> > > > - There were also concerns about forking discussions between JIRA and
> > > > Gerrit. This has been an issue in Spark, and we'd like to keep
> > > discussions
> > > > and issue tracking centralized.
> > > >
> > > > - Some talk about how to improve precommit. Right now it takes hours
> to
> > > run
> > > > the unit tests, which slows down patch iterations. One solution is
> > > running
> > > > tests in parallel (and even distributed). Previous distributed
> > > experiments
> > > > have done a full unit test run in a couple minutes, but it'd be a
> fair
> > > > amount of work to actually make this production ready.
> > > > - Also mention of putting in place more linting and static analysis.
> > > > Automating this will save reviewer time.
> > > >
> > > > Best,
> > > > Andrew
> > > >
> > > > On Mon, Jan 26, 2015 at 9:16 AM, Ted Yu <yuzhihong@gmail.com> wrote:
> > > >
> > > > > In some cases, contributor responded to review comments and
> attached
> > > > > patches addressing the comments.
> > > > >
> > > > > Later on, there was simply no response to the latest patch - even
> > with
> > > > > follow-on ping.
> > > > >
> > > > > I wish this aspect can be improved.
> > > > >
> > > > > Cheers
> > > > >
> > > > > On Sun, Jan 25, 2015 at 6:03 PM, Tsz Wo (Nicholas), Sze <
> > > > > s29752-hadoopgeneral@yahoo.com.invalid> wrote:
> > > > >
> > > > > > Hi contributors,
> > > > > > I would like to (re)start a discussion regrading to our patch
> > review
> > > > > > process.  A similar discussion has been happened in a the hadoop
> > > > private
> > > > > > mailing list, which is inappropriate.
> > > > > > Here is the problem:The patch available queues become longer
and
> > > > longer.
> > > > > > It seems that we never can catch up.  There are patches sitting
> in
> > > the
> > > > > > queues for years.  How could we speed up?
> > > > > > Regrads,Tsz-Wo
> > > > > >
> > > > >
> > > >
> > >
> > > --
> > > CONFIDENTIALITY NOTICE
> > > NOTICE: This message is intended for the use of the individual or
> entity
> > to
> > > which it is addressed and may contain information that is confidential,
> > > privileged and exempt from disclosure under applicable law. If the
> reader
> > > of this message is not the intended recipient, you are hereby notified
> > that
> > > any printing, copying, dissemination, distribution, disclosure or
> > > forwarding of this communication is strictly prohibited. If you have
> > > received this communication in error, please contact the sender
> > immediately
> > > and delete it from your system. Thank You.
> > >
> >
> > --
> > CONFIDENTIALITY NOTICE
> > NOTICE: This message is intended for the use of the individual or entity
> to
> > which it is addressed and may contain information that is confidential,
> > privileged and exempt from disclosure under applicable law. If the reader
> > of this message is not the intended recipient, you are hereby notified
> that
> > any printing, copying, dissemination, distribution, disclosure or
> > forwarding of this communication is strictly prohibited. If you have
> > received this communication in error, please contact the sender
> immediately
> > and delete it from your system. Thank You.
> >
>
> --
> CONFIDENTIALITY NOTICE
> NOTICE: This message is intended for the use of the individual or entity to
> which it is addressed and may contain information that is confidential,
> privileged and exempt from disclosure under applicable law. If the reader
> of this message is not the intended recipient, you are hereby notified that
> any printing, copying, dissemination, distribution, disclosure or
> forwarding of this communication is strictly prohibited. If you have
> received this communication in error, please contact the sender immediately
> and delete it from your system. Thank You.
>

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message