Re: [DISCUSS] Review-then-commit (RTC) vs Commit-then-review (CTR)
It's up to each community to decide what it's comfortable with, and then make sure the process is applied equally to everyone. In my experience, 'non-minor, trivial' is pretty nebulous. Does a 3,000-line IDE-driven refactor count as non-minor? What about a one line change in the heart of the scheduler that inadvertently changes a != to a ==? It's generally agreed that immediate fixes for broken builds (e.g., missing seimicolons that break the compiler, or the Python equivalent), can be checked in without reviews (they're usually attached to the original ticket). -jakob On 20 May 2016 at 09:23, siddharth anandwrote: > Can we suggest a +1 vote for non-minor/non-trivial commits, or do all > commits require a +1? Or is the "non-minor, non-trivial" wording too > nebulous. t feels like this would work better the more committers we have > given our different daily work schedules. > > -s > > On Thu, May 19, 2016 at 5:56 PM, Jakob Homan wrote: > >> On 18 May 2016 at 16:06, siddharth anand wrote: >> > Is there an automation we can use to ensure/enforce that PRs >> > and JIRAs receive the +1 binding (from committers) votes needed before a >> > merge can proceed >> >> Not that I know of. Since we're using github PRs, there might be some >> kind of workflow system that plugs into that, but I'm not aware of it. >> Honor code works well, and reverting commits work when it doesn't... >>
Re: [DISCUSS] Review-then-commit (RTC) vs Commit-then-review (CTR)
Can we suggest a +1 vote for non-minor/non-trivial commits, or do all commits require a +1? Or is the "non-minor, non-trivial" wording too nebulous. t feels like this would work better the more committers we have given our different daily work schedules. -s On Thu, May 19, 2016 at 5:56 PM, Jakob Homanwrote: > On 18 May 2016 at 16:06, siddharth anand wrote: > > Is there an automation we can use to ensure/enforce that PRs > > and JIRAs receive the +1 binding (from committers) votes needed before a > > merge can proceed > > Not that I know of. Since we're using github PRs, there might be some > kind of workflow system that plugs into that, but I'm not aware of it. > Honor code works well, and reverting commits work when it doesn't... >
Re: [DISCUSS] Review-then-commit (RTC) vs Commit-then-review (CTR)
Thanks for sharing this information. All, Please share any additional thoughts. In a few days, I will kick off a VOTE thread to decide our process. Mentors, Just curious! Is there an automation we can use to ensure/enforce that PRs and JIRAs receive the +1 binding (from committers) votes needed before a merge can proceed? Something like a drop-in client-side pre-git-push hook script for pushes to apache master. Or should we just follow an honor code. -s On Wed, May 18, 2016 at 6:14 PM, Jakob Homanwrote: > Hey- >The consensus approval link is referring to votes (e.g. releases, > new committers, new PMCers, etc) rather than code changes. A single > +1 is standard for code changes, as Chris describes. However, > projects are welcome to provide more fine-grained requirements as > well. For example, Hadoop is RTC (Review-then-commit) but also > features to be developed in branches that run on CTR > (Commit-then-review). Such a branch can only be merged back to master > with three binding +1s, as a counterweight to the possibility of > un-reviewed changes appearing. > > Historically, ASF operated on a CTR model. However, Hadoop and the > myriad projects that flowed from it, adopted RTC. I'm not aware of > any big-data/Hadoop/Kafka projects that currently use CTR. (This > comment's not an endorsement of either approach, just an observation) > >Additionally, the community can chose any time to change its > approach (via a vote). > -Jakob > > > On 18 May 2016 at 10:55, Chris Riccomini wrote: > > Hey Sid, > > > >> In the RTC case, we need 3 +1 binding (a.k.a. committer) votes > > > > This sounds very high. Usually one +1 (other than the person sending the) > > is normal in an RTC scenario. > > > >> In the CTR case, we may want a separate develop branch against which to > > run integration tests and merge to master only after those tests pass > > > > I would prefer not to have a separate branch, so if we feel like that's a > > requirement for CTR, then I'd prefer RTC. If we're comfortable committing > > to master, though, then I'm fine either way. We have quite a few active > > committers, so waiting 24h for a review seems fine. > > > > Basically, I don't have a strong preference either way, except that I > > strongly prefer not to have a separate development branch. If you force > me > > to pick, I'd pick RTC. I find that RTC encourages a shared understanding > of > > the code, which is useful. > > > > Cheers, > > Chris > > > > On Tue, May 17, 2016 at 8:10 PM, siddharth anand > wrote: > > > >> Folks, > >> It's a good time for us to discuss whether we will adopt a RTC or CTR > >> policy towards software changes. > >> > >> In the RTC case, we need 3 +1 binding (a.k.a. committer) votes and 0 > >> binding vetos for any change as RTC requires consensus approval: > >> > >>- http://www.apache.org/foundation/glossary.html#ReviewThenCommit > >>- http://www.apache.org/foundation/glossary.html#ConsensusApproval > >> > >> In the CTR case, we operate by lazy consensus, which many of the > committers > >> have already exercised for the recent Committer/PPMC vote. In the CTR > case, > >> we may want a separate develop branch against which to run integration > >> tests and merge to master only after those tests pass. I'm curious about > >> this approach and would like to understand how well this is supported > via > >> infra tools, automation, and documentation. > >> > >>- http://www.apache.org/foundation/glossary.html#CommitThenReview > >>- http://www.apache.org/foundation/glossary.html#LazyConsensus > >> > >> I'm also curious if we need to strictly adopt one of these or whether we > >> can roll our own - e.g. a +1 binding for RTC for example. > >> > >> Mentors, > >> Any guidance here? > >> > >> -s > >> >
Re: [DISCUSS] Review-then-commit (RTC) vs Commit-then-review (CTR)
Hey- The consensus approval link is referring to votes (e.g. releases, new committers, new PMCers, etc) rather than code changes. A single +1 is standard for code changes, as Chris describes. However, projects are welcome to provide more fine-grained requirements as well. For example, Hadoop is RTC (Review-then-commit) but also features to be developed in branches that run on CTR (Commit-then-review). Such a branch can only be merged back to master with three binding +1s, as a counterweight to the possibility of un-reviewed changes appearing. Historically, ASF operated on a CTR model. However, Hadoop and the myriad projects that flowed from it, adopted RTC. I'm not aware of any big-data/Hadoop/Kafka projects that currently use CTR. (This comment's not an endorsement of either approach, just an observation) Additionally, the community can chose any time to change its approach (via a vote). -Jakob On 18 May 2016 at 10:55, Chris Riccominiwrote: > Hey Sid, > >> In the RTC case, we need 3 +1 binding (a.k.a. committer) votes > > This sounds very high. Usually one +1 (other than the person sending the) > is normal in an RTC scenario. > >> In the CTR case, we may want a separate develop branch against which to > run integration tests and merge to master only after those tests pass > > I would prefer not to have a separate branch, so if we feel like that's a > requirement for CTR, then I'd prefer RTC. If we're comfortable committing > to master, though, then I'm fine either way. We have quite a few active > committers, so waiting 24h for a review seems fine. > > Basically, I don't have a strong preference either way, except that I > strongly prefer not to have a separate development branch. If you force me > to pick, I'd pick RTC. I find that RTC encourages a shared understanding of > the code, which is useful. > > Cheers, > Chris > > On Tue, May 17, 2016 at 8:10 PM, siddharth anand wrote: > >> Folks, >> It's a good time for us to discuss whether we will adopt a RTC or CTR >> policy towards software changes. >> >> In the RTC case, we need 3 +1 binding (a.k.a. committer) votes and 0 >> binding vetos for any change as RTC requires consensus approval: >> >>- http://www.apache.org/foundation/glossary.html#ReviewThenCommit >>- http://www.apache.org/foundation/glossary.html#ConsensusApproval >> >> In the CTR case, we operate by lazy consensus, which many of the committers >> have already exercised for the recent Committer/PPMC vote. In the CTR case, >> we may want a separate develop branch against which to run integration >> tests and merge to master only after those tests pass. I'm curious about >> this approach and would like to understand how well this is supported via >> infra tools, automation, and documentation. >> >>- http://www.apache.org/foundation/glossary.html#CommitThenReview >>- http://www.apache.org/foundation/glossary.html#LazyConsensus >> >> I'm also curious if we need to strictly adopt one of these or whether we >> can roll our own - e.g. a +1 binding for RTC for example. >> >> Mentors, >> Any guidance here? >> >> -s >>
Re: [DISCUSS] Review-then-commit (RTC) vs Commit-then-review (CTR)
Hey Sid, > In the RTC case, we need 3 +1 binding (a.k.a. committer) votes This sounds very high. Usually one +1 (other than the person sending the) is normal in an RTC scenario. > In the CTR case, we may want a separate develop branch against which to run integration tests and merge to master only after those tests pass I would prefer not to have a separate branch, so if we feel like that's a requirement for CTR, then I'd prefer RTC. If we're comfortable committing to master, though, then I'm fine either way. We have quite a few active committers, so waiting 24h for a review seems fine. Basically, I don't have a strong preference either way, except that I strongly prefer not to have a separate development branch. If you force me to pick, I'd pick RTC. I find that RTC encourages a shared understanding of the code, which is useful. Cheers, Chris On Tue, May 17, 2016 at 8:10 PM, siddharth anandwrote: > Folks, > It's a good time for us to discuss whether we will adopt a RTC or CTR > policy towards software changes. > > In the RTC case, we need 3 +1 binding (a.k.a. committer) votes and 0 > binding vetos for any change as RTC requires consensus approval: > >- http://www.apache.org/foundation/glossary.html#ReviewThenCommit >- http://www.apache.org/foundation/glossary.html#ConsensusApproval > > In the CTR case, we operate by lazy consensus, which many of the committers > have already exercised for the recent Committer/PPMC vote. In the CTR case, > we may want a separate develop branch against which to run integration > tests and merge to master only after those tests pass. I'm curious about > this approach and would like to understand how well this is supported via > infra tools, automation, and documentation. > >- http://www.apache.org/foundation/glossary.html#CommitThenReview >- http://www.apache.org/foundation/glossary.html#LazyConsensus > > I'm also curious if we need to strictly adopt one of these or whether we > can roll our own - e.g. a +1 binding for RTC for example. > > Mentors, > Any guidance here? > > -s >