incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <ralph.go...@dslextreme.com>
Subject Re: RTC vs CTR (was: Concerning Sentry...)
Date Wed, 25 Nov 2015 21:32:54 GMT
1. What makes you think all bugs are caught during code reviews (they aren’t)?
2. What makes you think that code reviews after the commit are any less thorough than reviews
required before the commit?

If you don’t trust your community to do code reviews after you commit then there is a problem
in your community. Forcing a code review to occur first won’t fix that.

In a CTR world you can choose to do every piece of work on a branch and ask for a code review
before you commit. That is your choice.  But if you know that the code you are committing
is good because you have a thorough knowledge of your product you shouldn’t be forced to
have it reviewed before you can commit it.  I actually love the line in Kudu where it says
that automation insures quality. I am a big fan of that. In my experience having lots of tests
is the best way to insure stuff doesn’t get broken.

So how did you catch the bugs in your code in Flex?  Would you have preferred that they stay
on a branch for months so they could be reviewed before committing?  Despite how great people
say git is I have still had lots of problems resolving merge conflicts when the code isn’t
merged back quickly.  If I understand what you are saying you would prefer that Flex use RTC
because you don’t trust your fellow committers to review your code.  That is a community
problem that needs to be fixed. Forcing them to review the code first isn’t the proper way
to fix it.

Ralph

> On Nov 25, 2015, at 2:00 PM, Harbs <harbs.lists@gmail.com> wrote:
> 
> If a review is required for non-code changes to the main branch, then I agree.
> 
> I’m sure you agree that reviews on code make for less bugs. We all make mistakes and
can overlook things. It seems kind of extreme to assume that this kind of required review
is all about control. Since anyone who can commit can review, it’s kind of hard for me to
swallow that.
> 
> I assume your logic is that the reviews can come after the commit. Sure. But what if
it doesn’t?
> 
> Case in point: I made some pretty major changes to TLF in Flex which constituted a number
of months of work. I’m willing to bet that not every commit I did was checked by others.
I did a decent job, but there were a few regressive bugs in my code, and I accidentally reverted
some code in my commit as well. In a workflow where my code would have to get one or more
+1s before I committed it to the main branch, it’s likely that the reverted commit (at the
least) would have been caught.
> 
> I would actually welcome knowing someone looked over my code for a sanity check.
> 
> Harbs
> 
> On Nov 25, 2015, at 10:49 PM, Greg Stein <gstein@gmail.com> wrote:
> 
>> That is pretty normal operation in both styles of workflow. My concern is
>> with trunk/master. Is a committer trusted enough to make changes directly?
>> 
>> If all meaningful changes (ie. changing APIs and algorithms, not just
>> fixing typos w/o review) are not trusted, and require review/permission,
>> then I'm against that.
>> 
>> It is good practice to put potentially disruptive code onto a branch while
>> it is developed, then merge it when complete. Trusting a committer to ask
>> for review before the merge is great. Requiring it, less so.
>> 
>> But RTC on trunk/master is harmful.
>> 
>> Cheers,
>> -g
>> 
>> On Wed, Nov 25, 2015 at 2:44 PM, Harbs <harbs.lists@gmail.com> wrote:
>> 
>>> What about commit to feature/bug brach, review and then commit to main
>>> branch?
>>> 
>>> Is that CTR or RTC in your book?
>>> 
>>> On Nov 25, 2015, at 10:42 PM, Greg Stein <gstein@gmail.com> wrote:
>>> 
>>>> I object to Lucene's path, too. A committer's judgement is not trusted
>>>> enough to make a change without upload/review. They need permission first
>>>> (again: to use your term; it works great).
>>>> 
>>>> On Wed, Nov 25, 2015 at 2:39 PM, Upayavira <uv@odoko.co.uk> wrote:
>>>> 
>>>>> Some setups that people call RTC are actually CTR in your nomenclature,
>>>>> so we could be talking cross-purposes. That's all I'm trying to avoid.
>>>>> E.g. Lucene - everything happens in JIRA first (upload patch, wait for
>>>>> review), but once that has happened, you are free to commit away. So
>>>>> strictly, it is RTC, but not seemingly in the sense you are objecting
>>>>> to.
>>>>> 
>>>>> Upayavira
>>>>> 
>>>>> On Wed, Nov 25, 2015, at 08:35 PM, Greg Stein wrote:
>>>>>> I think this is a distraction. You said it best the other day: RTC
>>>>>> implies
>>>>>> the need for "permission" before making a change to the codebase.
>>>>>> Committers are not trusted to make a judgement on whether a change
>>> should
>>>>>> be made.
>>>>>> 
>>>>>> CTR trusts committers to use their judgement. RTC distrusts committers,
>>>>>> and
>>>>>> makes them seek permission [though one of several mechanisms].
>>>>>> 
>>>>>> -g
>>>>>> 
>>>>>> On Wed, Nov 25, 2015 at 10:47 AM, Upayavira <uv@odoko.co.uk>
wrote:
>>>>>> 
>>>>>>> Not replying to this mail specifically, but to the thread in
>>> general...
>>>>>>> 
>>>>>>> People keep using the terms RTC and CTR as if we all mean the
same
>>>>>>> thing. Please don't. If you must use these terms, please define
what
>>>>> you
>>>>>>> mean by them.
>>>>>>> 
>>>>>>> CTR is a less ambiguous term - I'd suggest we all assume that
"commit"
>>>>>>> means a push to a version control system.
>>>>>>> 
>>>>>>> However, RTC seems to mean many things - from "push to JIRA for
review
>>>>>>> first, wait a bit, then commit to VCS" through "push to JIRA,
and once
>>>>>>> you have sufficient +1 votes, you can commit" to "push to JIRA
for a
>>>>>>> review, then another committer must commit it".
>>>>>>> 
>>>>>>> If we're gonna debate RTC, can we please describe which of these
we
>>> are
>>>>>>> talking about (or some other mechanism that I haven't described)?
>>>>>>> Otherwise, we will end up endlessly debating over the top of
each
>>>>> other.
>>>>>>> 
>>>>>>> Upayavira
>>>>>>> 
>>>>>>> On Wed, Nov 25, 2015, at 09:28 AM, Harbs wrote:
>>>>>>>> AIUI, there’s two ways to go about RTC which is easier
in Git:
>>>>>>>> 1) Working in feature/bug fix branches. Assuming RTC only
applies to
>>>>> the
>>>>>>>> main branch, changes are done in separate branches where
commits do
>>>>> not
>>>>>>>> require review. The feature/bug fix branch is then only merged
back
>>>>> in
>>>>>>>> after it had a review. The reason this is easier is because
>>>>> branching and
>>>>>>>> merging is almost zero effort in Git. Many Git workflows
don’t work
>>>>> on
>>>>>>>> the main branch anyway, so this is a particularly good fit
for those
>>>>>>>> workflows.
>>>>>>>> 2) Pull requests. Using pull requests, all changes can be
pulled in
>>>>> with
>>>>>>>> a single command.
>>>>>>>> 
>>>>>>>> I’ve personally never participated in RTC (unless you count
Github
>>>>>>>> projects and before I was a committer in Flex), so it could
be I’m
>>>>>>>> missing something.
>>>>>>>> 
>>>>>>>> Of course there’s nothing to ENFORCE that the commit is
not done
>>>>> before a
>>>>>>>> review, but why would you want to do that? That’s where
trust comes
>>>>> to
>>>>>>>> play… ;-)
>>>>>>>> 
>>>>>>>> Harbs
>>>>>>>> 
>>>>>>>> On Nov 25, 2015, at 4:08 AM, Konstantin Boudnik <cos@apache.org>
>>>>> wrote:
>>>>>>>> 
>>>>>>>>> I don't think Git is particularly empowering RTC - there's
nothing
>>>>> in
>>>>>>> it that
>>>>>>>>> requires someone to look over one's shoulder.
>>>>>>>> 
>>>>>>> 
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: general-unsubscribe@incubator.apache.org
>>>>>>> For additional commands, e-mail: general-help@incubator.apache.org
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: general-unsubscribe@incubator.apache.org
>>>>> For additional commands, e-mail: general-help@incubator.apache.org
>>>>> 
>>>>> 
>>> 
>>> 
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: general-unsubscribe@incubator.apache.org
>>> For additional commands, e-mail: general-help@incubator.apache.org
>>> 
>>> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@incubator.apache.org
For additional commands, e-mail: general-help@incubator.apache.org


Mime
View raw message