Re: RTC/CTR Discussion

[Joe Witt]

For sure.

Code can always evolve and if a commit happens that needs some refinement
all is fine.  In essence ctr is always available.  For us adopting RTC it
means, in my opinion, that you should obtain an independent opinion that it
is good to go.  As new folks contribute it stokes engagement and even
mentoring and as veterans of the project contribute it encourages shared
understanding.

I see it as a sort of rTCtr (case intentional).

Provably worth documenting the approach either way on a wiki.

Thanks
Joe

On Jul 24, 2016 10:59 AM, "Ellison Anne Williams" 
wrote:

> This was raised on a recent PR commit; bringing it here for discussion:
>
> It's my understanding the members of the PMC (Tim, Suneel, etc) are also
> committers and able to perform a review in a RTC scenario.
>
> Thoughts?
>
> On Fri, Jul 22, 2016 at 10:05 PM, Josh Elser  wrote:
>
> > I lean towards just needing a +1 to commit and still doing that myself.
> >
> > Just need to be clear however is decided.
> >
> >
> > Ellison Anne Williams wrote:
> >
> >> I don't see point in having to get a +1 from another committer for
> fixing
> >> trivial items such as merge conflicts, build breaks, etc. I also would
> >> through trivial updates to the website like fixing typos, broken
> >> hyperlinks, adding hyperlinks, etc.
> >>
> >> My interpretation of RTC is that the author can commit their own changes
> >> after receiving +1 from a reviewer (another committer).
> >>
> >> On Fri, Jul 22, 2016 at 4:15 AM, Tim Ellison
> >> wrote:
> >>
> >> On 22/07/16 05:50, Andy LoPresto wrote:
> >>>
>  One of the nice effects of RTC is that the community gets an
>  opportunity to gently enforce code convention and prevent rapid build
>  up of technical debt. Something else I've witnessed is that as the
>  community grows, non-committers feel more comfortable submitting PRs
>  because they've seen the same review process applied regardless of
>  the submitter's status.
> 
>  Apologies if I missed these points on an earlier thread.
> 
> >>> Sure, I don't feel strongly either way -- though I would expect a
> >>> committer to be able to fix trivial items like a simple merge conflict,
> >>> or backout a build breaking change without always having to get a
> >>> backing +1 from elsewhere.
> >>>
> >>> Out of curiosity, does folks' interpretation of RTC mean that the
> author
> >>> never commits their own changes? i.e. the reviewer always commits (as
> >>> the original author)?
> >>>
> >>> Regards,
> >>> Tim
> >>>
> >>>
> >>
>

Re: RTC/CTR Discussion

[Ellison Anne Williams]

This was raised on a recent PR commit; bringing it here for discussion:

It's my understanding the members of the PMC (Tim, Suneel, etc) are also
committers and able to perform a review in a RTC scenario.

Thoughts?

On Fri, Jul 22, 2016 at 10:05 PM, Josh Elser  wrote:

> I lean towards just needing a +1 to commit and still doing that myself.
>
> Just need to be clear however is decided.
>
>
> Ellison Anne Williams wrote:
>
>> I don't see point in having to get a +1 from another committer for fixing
>> trivial items such as merge conflicts, build breaks, etc. I also would
>> through trivial updates to the website like fixing typos, broken
>> hyperlinks, adding hyperlinks, etc.
>>
>> My interpretation of RTC is that the author can commit their own changes
>> after receiving +1 from a reviewer (another committer).
>>
>> On Fri, Jul 22, 2016 at 4:15 AM, Tim Ellison
>> wrote:
>>
>> On 22/07/16 05:50, Andy LoPresto wrote:
>>>
 One of the nice effects of RTC is that the community gets an
 opportunity to gently enforce code convention and prevent rapid build
 up of technical debt. Something else I've witnessed is that as the
 community grows, non-committers feel more comfortable submitting PRs
 because they've seen the same review process applied regardless of
 the submitter's status.

 Apologies if I missed these points on an earlier thread.

>>> Sure, I don't feel strongly either way -- though I would expect a
>>> committer to be able to fix trivial items like a simple merge conflict,
>>> or backout a build breaking change without always having to get a
>>> backing +1 from elsewhere.
>>>
>>> Out of curiosity, does folks' interpretation of RTC mean that the author
>>> never commits their own changes? i.e. the reviewer always commits (as
>>> the original author)?
>>>
>>> Regards,
>>> Tim
>>>
>>>
>>

Re: RTC/CTR Discussion

[Josh Elser]

I lean towards just needing a +1 to commit and still doing that myself.

Just need to be clear however is decided.

Ellison Anne Williams wrote:

I don't see point in having to get a +1 from another committer for fixing
trivial items such as merge conflicts, build breaks, etc. I also would
through trivial updates to the website like fixing typos, broken
hyperlinks, adding hyperlinks, etc.

My interpretation of RTC is that the author can commit their own changes
after receiving +1 from a reviewer (another committer).

On Fri, Jul 22, 2016 at 4:15 AM, Tim Ellison  wrote:


On 22/07/16 05:50, Andy LoPresto wrote:

One of the nice effects of RTC is that the community gets an
opportunity to gently enforce code convention and prevent rapid build
up of technical debt. Something else I've witnessed is that as the
community grows, non-committers feel more comfortable submitting PRs
because they've seen the same review process applied regardless of
the submitter's status.

Apologies if I missed these points on an earlier thread.

Sure, I don't feel strongly either way -- though I would expect a
committer to be able to fix trivial items like a simple merge conflict,
or backout a build breaking change without always having to get a
backing +1 from elsewhere.

Out of curiosity, does folks' interpretation of RTC mean that the author
never commits their own changes? i.e. the reviewer always commits (as
the original author)?

Regards,
Tim

Re: RTC/CTR Discussion

[Joe Witt]

+1.


On Tue, Jul 19, 2016 at 5:27 PM, Ellison Anne Williams
 wrote:
> It seems that we could give RTC a shot, with one reviewer posting a +1 (or
> equivalent) comment on a pull request before it is accepted, and switch
> back to CTR if RTC became too burdensome.
>
> It seems healthy to foster explicit accountability and to give others the
> opportunity to discuss changes before they are committed. This may become
> too much for small commits, especially to the website, but we can always
> adopt CTR (or a combination of the two for certain situations).
>
> Thoughts?
>
> Thanks!
>
> On Mon, Jul 18, 2016 at 9:22 AM, Suneel Marthi  wrote:
>
>> thanks Tim, this is a great perspective indeed, like the way u put it.
>>
>> On Mon, Jul 18, 2016 at 6:50 AM, Tim Ellison 
>> wrote:
>>
>> > On 17/07/16 19:31, Suneel Marthi wrote:
>> > > +1 to RTC, this is what's presently done on Flink, Mahout and few other
>> > > projects I had seen.
>> > >
>> > > Its usually a committer reviewing the patch, providing feedback, and
>> > > finally committing it.
>> > > Whether a pull request gets a +1 from another committer or not is
>> > entirely
>> > > up to the project and could be decided on a case-by-case basis.
>> > >
>> > > A complex change or design might need more reviewers while a simple
>> > change
>> > > could be good to commit without additional reviews.
>> >
>> > By way of balance, I'll offer up the argument for CTR with tweaks...
>> >
>> > CTR has a number of advantages too across a number of different
>> scenarios.
>> >
>> > First thing to say is that committers are usually trusted to "do the
>> > right thing", so further reviews of a committer's work are generally
>> > introduced only when additional checks-and-balances are required.
>> > Working with CTR allows the committer to just get on with it, on the
>> > understanding that numerous eyes are watching the changes and will raise
>> > any problems that they see.
>> >
>> > Of course, the code is all in version control, so where a committer has
>> > changed something that may be a problem for others, it is easy to roll
>> > files back to a point where the objection is being raised, and walk
>> > forwards again during a review / discussion.
>> >
>> > CTR allows for faster development (no need to wait for reviews) and
>> > arguably more frequent committing (no tendency to save up chunks for
>> > block review before they hit the repo).  Many eyes watching small
>> > commits may be better than a nominated review of a large piece of work.
>> >
>> > That said, there is a place for RTC - and when working towards a release
>> > it is helpful to chill the codebase (no new features) and test, then
>> > freeze (no bug fixes without RTC) before making the release, to ensure
>> > stability.
>> >
>> > Regards,
>> > Tim
>> >
>> >
>>

Re: RTC/CTR Discussion

[Tim Ellison]

On 17/07/16 19:31, Suneel Marthi wrote:
> +1 to RTC, this is what's presently done on Flink, Mahout and few other
> projects I had seen.
> 
> Its usually a committer reviewing the patch, providing feedback, and
> finally committing it.
> Whether a pull request gets a +1 from another committer or not is entirely
> up to the project and could be decided on a case-by-case basis.
> 
> A complex change or design might need more reviewers while a simple change
> could be good to commit without additional reviews.

By way of balance, I'll offer up the argument for CTR with tweaks...

CTR has a number of advantages too across a number of different scenarios.

First thing to say is that committers are usually trusted to "do the
right thing", so further reviews of a committer's work are generally
introduced only when additional checks-and-balances are required.
Working with CTR allows the committer to just get on with it, on the
understanding that numerous eyes are watching the changes and will raise
any problems that they see.

Of course, the code is all in version control, so where a committer has
changed something that may be a problem for others, it is easy to roll
files back to a point where the objection is being raised, and walk
forwards again during a review / discussion.

CTR allows for faster development (no need to wait for reviews) and
arguably more frequent committing (no tendency to save up chunks for
block review before they hit the repo).  Many eyes watching small
commits may be better than a nominated review of a large piece of work.

That said, there is a place for RTC - and when working towards a release
it is helpful to chill the codebase (no new features) and test, then
freeze (no bug fixes without RTC) before making the release, to ensure
stability.

Regards,
Tim