I like Larry's idea. I have used a similar approach in small commercial projects with good results
However, it creates some procedural challenges: The advantage of the simple CTR/RTC dichotomy is that it is easily enforced by the mechanics of permissions. Under CTR, everyone has permission to commit. Under RTC very few have commit permission, so it is structurally enforced that someone at least claims to review the pull request. If we implement the hybrid approach we could need to structurally implement RTC, then the reviewer, as a matter of policy, simply has to merge the pull request -- effectively rubber-stamping the review. This creates a burden on the person who has to make the merges. This burden could be quite large if there is a lot of activity. If we take the opposite approach and trust people to make direct commits for bug fixes but always create pull requests for anything bigger than a bug fix, it is a pretty sure thing that the rule will not be followed. It goes wrong for a variety of reasons ranging from simply forgetting to malice. Policing the behavior becomes a significant task in itself. Does anybody have other ideas about how we would implement Larry's strategy? -Charles On Tue, Dec 8, 2015 at 4:25 AM, larry mccay <[email protected]> wrote: > My preference would actually be a hybrid with some simple guidelines: > > * Overall CTR - at least while we are developing critical mass and > functionality - however... > * Architectural, security related and largish patches should have a review > - if for nothing else but to communicate the intents clearly > * Bug fixes and minor changes should not require review but we should feel > free to request one at any time > > thoughts? > > On Tue, Dec 8, 2015 at 1:14 AM, Owen O'Malley <[email protected]> wrote: > > > Welcome everyone to the Metron project. > > > > One of the first points that we need to decide on is how you'd like to > run > > your development. In particular, most Apache projects use either > > Commit-Then-Review or Review-Then-Commit. Either one works and they each > > have pluses and minuses and unfortunately it can turn in to a religious > war > > of the vi vs emacs level. *grin* > > > > Commit-Then-Review lets committers make changes to the git repository and > > everyone watches the commits@metron list review the changes. > > > > Review-Then-Commit requires any changes to be uploaded to github or Jira > > and +1'ed by a different committer before it gets pushed to Metron's > master > > branch. > > > > The advantages are: > > > > * CTR > > * Patches by committers don't get blocked waiting for review. > > * It enables the project to move faster. > > > > * RTC > > * All changes are looked over before they are pushed, so some problems > > caught beforehand. > > * Less churn on master branch as review comments force changes. > > > > Thoughts? > > > > Owen > > >
