On 07/05/2015 15:24, Dan Smith wrote: > +1 For review then commit > > +1 For designating some maintainers for certain modules - as long as there > are no components with only 1 maintainer. > > Also, I think runnning all the tests for any commit that affects source > should be a requirement.
>From which I wonder, can we set up branch based CI? That would take any uncertainty out and remove the burden of a reviewer to run the tests. > Should we require any period of time for additional review comments, or can > the commit be merged as soon as it has been approved by enough committers? That's an interesting question. A cooling off period might be good. Also worth considering what happens in the event a proposal is not reviewed for a while. > -Dan > > On Thu, May 7, 2015 at 7:13 AM, Anthony Baker <[email protected]> wrote: > >> Agree that RTC is really important. In addition, we should consider that >> some changes require specific knowledge and context (I’m thinking of you >> AbstractRegionMap). Note that I’m not advocating for code ownership. >> Spark [1] uses this approach: >> >> "For certain modules, changes to the architecture and public API should >> also be reviewed by a maintainer for that module (which may or may not be >> the same as the main reviewer) before being merged. The PMC has designated >> the following maintainers…” >> >> Changes to public API’s or core internals would fall into this category. >> Thoughts? >> >> >> Anthony >> >> [1] https://cwiki.apache.org/confluence/display/SPARK/Committers >> >> >> >>> On May 7, 2015, at 3:38 AM, Justin Erenkrantz <[email protected]> >> wrote: >>> >>> One question that we need to discuss is whether every merge is RTC >>> (Review-than-Commit) or CTR (Commit-than-Review). >>> >>> My take is that we should start with RTC and, if the review process gets >> in >>> the way of innovation, then we go to CTR. But, until everyone learns the >>> rules of the road, I think RTC is justified. Under RTC rules, all >> commits >>> should be reviewed (+1) by three committers before being merged. (If you >>> are a committer, then two others are needed.). Any committer can veto >> (-1) >>> a patch - which should cause a discussion about resolving the veto. >>> >>> So, #1 - your suggestion sounds right with the need for three committers >> to >>> approve before merge to develop. >>> >>> For #2, I think it should be a separate branch and require 3 signoffs for >>> now. >>> >>> As the project matures, "obvious" commits can be CTR. >>> >>> My $.02. -- justin >>> On May 7, 2015 5:44 AM, "Pid" <[email protected]> wrote: >>> >>>> Hi, >>>> >>>> >>>> Like it says, can we discuss how the review process will work? >>>> For these examples: >>>> >>>> >>>> 1. I would like to work on upgrading the Spring dependencies in gfsh. >>>> >>>> Proposed approach: file a JIRA, cut a feature branch, push it & then >> what? >>>> >>>> >>>> 2. I would like to add an entry to .gitignore (.idea/) >>>> >>>> Does this require a JIRA, a feature branch and a review? >>>> >>>> >>>> >>>> p >>>> >>>> -- >>>> >>>> [key:62590808] >>>> >> >> > -- [key:62590808]
