hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Nauroth <cnaur...@hortonworks.com>
Subject Re: Branch policy question
Date Wed, 23 Mar 2016 21:12:35 GMT
I think this is good technical justification for commit-then-review in a
feature branch, and I would be +1 to support it.

Just to briefly summarize one of my earlier comments, I see this as a way
to unblock solo development work and keep it moving in parallel while
rallying community members for reviews and any further communication.  I
don't see it as skirting process, because we still have the requirement
for binding +1s to get it into a shipping branch.  Since a feature branch
merge requires 3 +1s, it actually has the side effect of increasing the
burden for getting the code into a shipping branch.  There is a balance to
strike here.  I could see some medium-sized efforts going either way, and
ultimately it's up to the best judgment of the contributors and committers
on whether or not a feature branch is the best way to work.

BTW, for those not watching the JIRAs, HADOOP-12857 just got a committer
review, so I believe this is now just about HADOOP-12930.

--Chris Nauroth




On 3/23/16, 2:01 PM, "Karthik Kambatla" <kasha@cloudera.com> wrote:

>A feature branch seems reasonable for this work: multiple reviewable
>patches that are meaningful only after all of them get in. I guess most
>comments here that had concerns for a branch were primarily based on the
>initial reasoning of not being able to find a reviewer.
>
>Coming to RTC and CTR, I haven't seen any branch that used CTR. May be, we
>should consider allowing CTR. But, if it is hard to get reviews done for
>individual smaller patches, I would think it would be hard to find 3
>committers to review them later JIRA-wise or the combined patch.
>
>On Wed, Mar 23, 2016 at 12:32 PM, Allen Wittenauer <aw@apache.org> wrote:
>
>>
>> > On Mar 23, 2016, at 10:25 AM, Chris Nauroth <cnauroth@hortonworks.com>
>> wrote:
>> >
>> > 2. Apache feature branches: Sign-off may come from designated branch
>> > committers in addition to full committers.  It's OK to break the
>>branch
>> > for work in progress, but it must be fixed before a merge.  It's still
>> > review then commit though.
>>
>> ...
>>
>> > Allen, are there particular reasons that you favor #2 (but with
>> > commit-then-review) instead of #3 for your work on HADOOP-12857 and
>> > HADOOP-12930?  Maybe you want the patches on Apache infrastructure so
>>you
>> > can get the pre-commit sign-offs quickly?  Is it something else?
>>
>>         HADOOP-12930 is sort of a weird case. I¹ve only written some
>>test
>> code, but... ignoring unit tests and some variable name cleanup, there
>> isn¹t much code being truly added or modified.  However there is
>> significant code _movement_ due to some refactoring that needs to take
>> place.  As a result, this has some implications on reviewers, the
>>testing
>> infrastructure, and how it is committed:
>>
>> * A combined patch file will be huge and overwhelming since a lot of
>> effort will need to be spent actually finding the changes rather than
>> having them as logically separated units of work.
>> * As all of you know, precommit doesn't work properly if any 2 or more
>>of
>> hadoop-common, hadoop-hdfs,  hadoop-mapreduce-project, and a handful of
>> others are touched due to the time it takes to do the unit test runs.
>>Guess
>> how many of these are being touched? :)
>> * Committing piece meal means that feature-set wise, it could/would be
>> incomplete until the last block is pushed into place.  This definitely
>> needs to get committed as a logical set (either as many smaller commits
>>at
>> once or a rebased/merged single commit) in trunk.
>> * Being in a branch means that reviewers can checkout and test rather
>>than
>> having to apply a series of patches [despite how easy smart-apply-patch
>> makes it. ;) ]
>> * One of the big criticisms during the 9902 review was that it would
>>have
>> been better to do it as a feature branch due to the size. This is me
>>trying
>> to acquiesce to that request. (It¹s worthwhile pointing out that 9902
>> couldn¹t have been easily broken up, however, since hadoop-config.sh was
>> touched. That was a major Œbeak the world¹ moment.)
>>
>>         A big problem with the PMC¹s ruleset around feature branches is
>> that they are pretty much built around the idea that teams of people are
>> going to be working on a feature. Realistically, I don¹t see that being
>>the
>> case here (despite some mild interest from other committers in the
>>feature)
>> esp given I can probably finish this feature off in a few days.  That¹s
>>why
>> I¹m asking if CTR into the branch followed by RTM into trunk is a
>>Œlegal'
>> strategy here, since branch merges require reviews anyway.
>>
>>         Yes, I could make a branch in my own repo on gitlab, but I doubt
>> that qualifies as something that is actually reviewable by the ASF¹s
>> rulesŠ. which brings us back here anyway if I turn this into a giant
>>patch
>> file.


Mime
View raw message