On Wed, Apr 28, 2021 at 11:07 AM Johan Corveleyn <jcor...@gmail.com> wrote:
> On Sun, Apr 25, 2021 at 4:44 PM Nathan Hartman <hartman.nat...@gmail.com> > wrote: > > > > On Fri, Apr 23, 2021 at 9:40 AM Daniel Shahaf <d...@daniel.shahaf.name> > wrote: > > > > > > Nathan Hartman wrote on Thu, 22 Apr 2021 21:41 +00:00: > > > > Not knowing whether / how many people have reviewed a particular > > > > commit is, as was said elsewhere, a silent failure mode of the CTR > > > > (commit-then-review) convention. > > > > > > > > Do we want to try switching to a RTC (review-then-commit) convention? > > > > > > If we do switch to RTC, we might want to also retroactively ensure all > > > commits post 1.14.x's branching have been reviewed by at least two > pairs > > > of eyes each. > > > > > > However, I wonder whether there's a smaller change we can do first, > > > rather than a full-blown s/CTR/RTC/ flag day. For instance, how > > > about we agree, for the next N weeks, to make our commit reviews > > > explicit? I.e., to explicitly say "I've reviewed this commit and > > > found no issues"? (Call this Commit-Then-Explicit-Review.) > > > > I'm +1 to this idea. It sounds to me like a reasonable middle ground > > to gain some insights before making bigger workflow-altering changes. > > I'm doubtful that this will improve our bus factor for reviews, except > perhaps by drawing in some more reviewer-attention by seeing more of > it on the list. > > So far I'm mostly seeing reviews by DanielSh and Nathan. Switching > from CTR to CTER (or to RTC) will not really change that. It might > Yeup. It's just rearranging deck chairs, rather than bringing in new reviewers. Let's also note that the analysis was regarding "re: svn commit". Meaning a selected group of commits which were commented-upon. That completely leaves out all of the totally awesome super-duper commits that I've made which definitely don't need comments! *wink* I would posit there are quite a few commits that occur which simply don't need review. And if you say "only these two guys are doing review", then RTC implies that *all* commits are conditioned upon two people's attention. I cannot see anything good coming out of that. CTR is predicated on trust of our fellow committers. They will get it right. Get them active, remove roadblocks, and carry the project forward. CTR is also predicated on svn: we have version control! We can unwind things that break. With a mature project like svn, moving to RTC is just imposing additional constraints upon our community, and (personally) I don't see the *benefit*. Most commits do not need a review. That's because I (we?) trust our fellow community members. Leave the 90% commits as they are: CTR. -1 on any switch to RTC for trunk. Cheers, -g
