My concern was mostly about what to do in the face of conflicts, but it sounds like the consensus is that for a clean merge, with no conflicts or test breakage (or other concerns) a committer is free to push without any oversight which is fine by me.
[If/when the Mergbot comes into action, and runs more extensive tests than standard precommit, it might make sense to still go through that rather than debug bad merges discovered in postcommit tests.] On Wed, Oct 26, 2016 at 9:07 PM, Davor Bonaci <[email protected]> wrote: > +1 > > I concur it is fine to proceed with a downstream integration (master -> > feature branch -> sub-feature branch) without waiting for review for a > completely clean merge. Exactly as proposed -- I think there should still > be a pull request and comment saying it is a clean merge. (In some ideal > world, this would happen nightly by a tool automatically, but I think > that's not feasible in the short term.) > > I think other cases (upstream integration, merge conflict, any manual > action, etc.) should still wait for a normal review. > > On Wed, Oct 26, 2016 at 10:34 AM, Thomas Weise <[email protected]> wrote: > >> +1 >> >> For a merge from master to the feature branch that does not require extra >> changes, RTC does not add value. It actually delays and burns reviewer time >> (even mechanics need some) that "real" PRs could benefit from. If >> adjustments are needed, then the regular process kicks in. >> >> Thanks, >> Thomas >> >> >> On Wed, Oct 26, 2016 at 1:33 AM, Amit Sela <[email protected]> wrote: >> >> > I generally agree with Kenneth. >> > >> > While working on the SparkRunnerV2 branch, it was a pain - i avoided >> > frequent merges to avoid trivial PRs, but it cost me with very large and >> > non-trivial merges later. >> > I think that frequent merges for feature-branches should most of the time >> > be trivial (no conflicts) and a committer should be allowed to self-merge >> > once tests pass. >> > As for conflicts, even for the smallest once I'd go with review just so >> > it's very clear when self-merging is OK - we can always revisit this >> later >> > and further discuss if we think we can improve this process. >> > >> > I guess +1 from me. >> > >> > Thanks, >> > Amit. >> > >> > On Wed, Oct 26, 2016 at 8:10 AM Frances Perry <[email protected]> >> > wrote: >> > >> > > On Tue, Oct 25, 2016 at 9:44 PM, Jean-Baptiste Onofré <[email protected] >> > >> > > wrote: >> > > >> > > > Agree. When possible it would be great to have the branch merged on >> > > master >> > > > quickly, even when it's not fully ready. It would give more >> visibility >> > to >> > > > potential contributors. >> > > > >> > > >> > > This thread is about the opposite, I think -- merging master into >> feature >> > > branches regularly to prevent them from getting out of sync. >> > > >> > > As for increasing the visibility of feature branches, we have these new >> > > webpages: >> > > http://beam.incubator.apache.org/contribute/work-in-progress/ >> > > http://beam.incubator.apache.org/contribute/contribution- >> > > guide/#feature-branches >> > > with more changes coming in the basic SDK/Runner landing pages too. >> > > >> > >>
