hadoop-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eli Collins <...@cloudera.com>
Subject Re: HDFS-1623 branching strategy
Date Mon, 11 Jul 2011 21:35:14 GMT
On Fri, Jul 8, 2011 at 3:40 PM, Jakob Homan <jghoman@gmail.com> wrote:
> I also have concerns.  While those working on the 1073 branch haven't
> abused the commit-then-review process, the fact remains that the final
> merge to trunk is equivalent to a 1.3MB patch that makes far-reaching
> changes to the guts of the NN.

Each change was done individually with it's own jira, patch, and
review.  People can review the sub-tasks if they don't want to look at
the entire patch. The majority of the changes were reviewed before
they were committed, when that wasn't the case review feedback was
incorporated in follow-on patches. I don't see how this is any
different from eg how federation was developed.

> Asking the reviewers most experienced
> in this code to do such a herculean review such a merge is an
> unreasonable request.  So (again with no malice on anyone's part),
> we've ended up with a huge, far-reaching changeset that was committed
> under a liberal policy that's not accepted on trunk being merged into

Look at the sub-tasks on jira. This was not a liberal policy. All the
changes are done as individual jiras and with individual patches for
review.  The design doc was reviewed and discussed by a number of

> trunk based on a single +1 (per the bylaws as I read them, it's not
> necessary to have more than that to merge a branch to trunk).  This is
> not a good situation.

The vast majority of changes that go into HDFS (and MR2 from my
understanding) are reviewed by a single person and others besides
myself (eg Sanjay, Ivan, and atm)  have reviewed patches on 1073
sub-tasks, so I don't see what the concern is. The new design, code,
and *huge* increase in test coverage is a massive improvement over
what we have currently.

I agree, in general, it would be great to see more reviewers, of both
the individual changes and the final merge.


View raw message