Thanks Chris N for digging back on this details. The main concern on this question was started like ³Since it¹s nearly impossible for me to get timely reviews for some build and script changes Š.². So, Even for CTR process, review is needed thing at some point. I have one question here is CTR tracks for each commit level reviews even if they are later? Or we just commit, can review like bulk? I understand now it may be branch managers decision. I feel this bulk review may be problematic if it comes directly when merge vote comes. I am sure review with that merge patch ( bigger patch containing all the branch changes) may not be that efficient than smaller level patch reviews. It is ok, if there is a way and reviewer reviews for each commit later, even after commit with CTR process. But if commits get accumulated with out reviews and to review whole changes at once at merge time, it may be much harder to review as bulk. In this current discussion case also, much hard to get some reviewers to review for a bulk patch. Is it make sense to leave JIRAs open until they get reviewed even if you commit the patch with CTR in branches (may be JIRA state can show something like ³In Review²)? So, that patches will get reviewed before coming to merge votes.
Regards, Uma On 3/23/16, 6:56 PM, "larry mccay" <lmc...@apache.org> wrote: >Thanks for digging that up, Chris. >That is completely what I would have expected but began questioning it >given this thread. > >I think that Allen's use of a feature branch for this effort makes sense >and that he should have the freedom to choose his commit policy for the >branch. >The tricky part will be getting the reviews at the end but I would imagine >that it can be managed with some documentation, code review, tests and >instructions. > >On Wed, Mar 23, 2016 at 5:20 PM, Chris Nauroth <cnaur...@hortonworks.com> >wrote: > >> It's interesting to go back to the change in bylaws in 2011 that >> introduced the requirement for 3 binding +1s on a branch merge [1]. The >> text of that resolution suggests that it's supportive of >> commit-then-review if that's what the developers on the branch want to >>do. >> >> "Branches' commit requirements are determined by the branch maintainer >>and >> in this situation are often set up as commit-then-review." >> >> It would also be very much against the spirit of that resolution to >> combine it all down into a single patch file and get a single +1. >> >> "As such, there is no way to guarantee that the entire change set >>offered >> for trunk merge has had a second pair of eyes on it. Therefore, it is >> prudent to give that final merge heightened scrutiny, particularly since >> these branches often extensively affect critical parts of the system. >> Requiring three binding +1s does not slow down the branch development >> process, but does provide a better chance of catching bugs before they >> make their way to trunk." >> >> --Chris Nauroth >> >> [1] https://s.apache.org/iW1F >> >> >> >> On 3/23/16, 2:11 PM, "Steve Loughran" <ste...@hortonworks.com> wrote: >> >> > >> >> On 22 Mar 2016, at 18:23, Andrew Wang <andrew.w...@cloudera.com> >>wrote: >> >> >> >> A branch sounds fine, but how are we going to get 3 +1's to merge >>it? If >> >> it's hard to find one reviewer, seems even harder to find two. >> > >> >Given that only one +1 is needed to merge a non-branch patch, he could >>in >> >theory convert the entire branch into a single .patch for review. Not >> >that I'd encourage that, just observing that its possible >> > >> > >> >> >> >> On Tue, Mar 22, 2016 at 10:56 AM, Allen Wittenauer < >> >> allenwittena...@yahoo.com.invalid> wrote: >> >> >> >>> >> >>>> On Mar 22, 2016, at 10:49 AM, larry mccay <larry.mc...@gmail.com> >> >>>>wrote: >> >>>> >> >>>> That sounds like a reasonable approach and valid use of branches to >> >>>>me. >> >>>> >> >>>> Perhaps a set of functional tests could be provided/identified that >> >>>>would >> >>>> help the review process by showing backward compatibility along >>with >> >>>>new >> >>>> extensions for things like dynamic commands? >> >>>> >> >>> >> >>> This is going into trunk, so no need for backward >>compatibility. >> >>> >> > >> > >> >>
