You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@chukwa.apache.org by Bernd Fondermann <be...@googlemail.com> on 2010/09/13 21:58:02 UTC
[DISCUSS] Applying commit-then-review
Hi,
If I understand correctly, Chukwa is following the review-then-commit
(RTC) pattern: Before every commit, a patch gets posted to a JIRA and
only on positive feedback it is committed.
As far as I can see, this is inherited from Hadoop's policies.
However, most projects at the ASF apply commit-then-review (CTR). CTR
has the advantage of being more agile, requiring less work (creating
issue, patch file, attaching it, waiting for feedback etc.) while
providing full oversight:
Every commit is reviewed by other committers after it happened, can be
discussed, reverted, improved etc. as a 'work in progress'.
It is best practice in CTR-mode to selectively use RTC, e.g. for big
patches or for potentially delicate commits.
I think Chukwa would profit from changing to CTR, so I'd like to know
what you think about it.
Thanks,
Bernd
Re: [DISCUSS] Applying commit-then-review
Posted by Bill Graham <bi...@gmail.com>.
I think this is a good discussion to have, even if we decide not to
change our process.
> The svn commit log and the chukwa-commit mailing list give you an easy
way to access patches.
I like the possibility of being able to commit without making a patch
file and attaching it to a JIRA, so digging into this a bit more...
Looking at "Subversion Commits" tab, let's use Eric's large CHUKWA-444
commit as an example. If he hadn't made a patch file, would there be
an easy way for a Chukwa user to get a patch of the diff from JIRA
(i.e., without having to check out the source and pipe an svn diff
between the revisions to a file)? If so, that would be ideal.
On Tue, Sep 14, 2010 at 3:17 AM, Bernd Fondermann
<be...@googlemail.com> wrote:
> On Mon, Sep 13, 2010 at 22:50, Bill Graham <bi...@gmail.com> wrote:
>> I can see advantages to both CTR and RTC. For bugs and even modest
>> changes, yes CTR seems fine. Recent examples where I would have
>> preferred to do that include:
>>
>> https://issues.apache.org/jira/browse/CHUKWA-517
>> https://issues.apache.org/jira/browse/CHUKWA-518
>> https://issues.apache.org/jira/browse/CHUKWA-519
>>
>> Bigger changes though, especially ones that have an external face on
>> them, like the recent REST API features, are good to have more of a
>> feedback gathering RTC phase, like these:
>>
>> https://issues.apache.org/jira/browse/CHUKWA-515
>> https://issues.apache.org/jira/browse/CHUKWA-520
>>
>> A few questions a have though about CTR:
>>
>> - Bernd, from your initial comments it sounds like in some cases you
>> don't even have a JIRA filed with the commit. Is that the case? I've
>> always thought that a commit should really have a JIRA just for
>> tracking, but I can change that thinking. Can you elaborate on how you
>> see JIRA playing into CTR?
>
> Before we had JIRA at the ASF (only bugzilla, which is no fun at all
> IMHO), we did CTR without filing issues, of course.
> Sometimes, patches where attached on-list.
> Today, most projects indeed file a JIRA for everything, to benefit
> from fully JIRA-generated release notes etc.
>
> CTR works great with JIRA:
> + Create JIRA issue
> + commit a patch, using the JIRA name ("CHUKWA-1234") in the commit log message
> + JIRA automatically detects and attaches the svn change log to the
> issue (see the "subversion commits" tab in JIRA)
> (+ iterate with more commits)
> + Close the issue (, reopen etc.)
>
>> - Similarly, it sounds like patches aren't needed with CTR. The
>> downside of this is that others can't easily apply the patch to a
>> release distribution and rebuild (something I do all the time). I'd
>> think if it's worth committing, it's worth having a patch file for
>> others to apply. Any thoughts around this?
>
> The svn commit log and the chukwa-commit mailing list give you an easy
> way to access patches.
>
>> I generally like the discussion around how to become more agile
>> though, since I see many benefits to be had there. I recently skimmed
>> some of the CTR discussion on the apache site and thought to myself
>> that that sounds different than what we're doing.
>
> Well, with this discussion thread I just meant to spin CTR. If RTC (or
> any relaxed derivative thereof) is the way *you* as committers want to
> go ahead, I definitively support it.
> I don't mean to discrupt any well-established process just for the
> sake of disruption.
>
> Bernd
>
>> On Mon, Sep 13, 2010 at 1:14 PM, Ariel Rabkin <as...@gmail.com> wrote:
>>>
>>> Our methodology is actually some hybrid; something like "wait for
>>> potential review", then commit. Usually I'll commit after a few days
>>> if nobody objected or commented. (I try to say explicitly when I'll
>>> time out waiting for comments.) And most of the patch review is
>>> fairly cursory.
>>>
>>> I agree that the full rigor of RTC is probably unnecessary. I do like
>>> it, however, for bigger changes, particularly those that alter the
>>> architecture or user visible feature set.
>>>
>>> Is it reasonable to say "for bugs, CTR is fine, and only file a JIRA
>>> and wait for review if it's going to break something"?
>>>
>>> --Ari
>>>
>>> On Mon, Sep 13, 2010 at 12:58 PM, Bernd Fondermann
>>> <be...@googlemail.com> wrote:
>>> > Hi,
>>> >
>>> > If I understand correctly, Chukwa is following the review-then-commit
>>> > (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
>>> > only on positive feedback it is committed.
>>> > As far as I can see, this is inherited from Hadoop's policies.
>>> > However, most projects at the ASF apply commit-then-review (CTR). CTR
>>> > has the advantage of being more agile, requiring less work (creating
>>> > issue, patch file, attaching it, waiting for feedback etc.) while
>>> > providing full oversight:
>>> > Every commit is reviewed by other committers after it happened, can be
>>> > discussed, reverted, improved etc. as a 'work in progress'.
>>> > It is best practice in CTR-mode to selectively use RTC, e.g. for big
>>> > patches or for potentially delicate commits.
>>> >
>>> > I think Chukwa would profit from changing to CTR, so I'd like to know
>>> > what you think about it.
>>> >
>>> > Thanks,
>>> >
>>> > Bernd
>>> >
>>>
>>>
>>>
>>> --
>>> Ari Rabkin asrabkin@gmail.com
>>> UC Berkeley Computer Science Department
>>
>
Re: [DISCUSS] Applying commit-then-review
Posted by Bernd Fondermann <be...@googlemail.com>.
On Mon, Sep 13, 2010 at 22:50, Bill Graham <bi...@gmail.com> wrote:
> I can see advantages to both CTR and RTC. For bugs and even modest
> changes, yes CTR seems fine. Recent examples where I would have
> preferred to do that include:
>
> https://issues.apache.org/jira/browse/CHUKWA-517
> https://issues.apache.org/jira/browse/CHUKWA-518
> https://issues.apache.org/jira/browse/CHUKWA-519
>
> Bigger changes though, especially ones that have an external face on
> them, like the recent REST API features, are good to have more of a
> feedback gathering RTC phase, like these:
>
> https://issues.apache.org/jira/browse/CHUKWA-515
> https://issues.apache.org/jira/browse/CHUKWA-520
>
> A few questions a have though about CTR:
>
> - Bernd, from your initial comments it sounds like in some cases you
> don't even have a JIRA filed with the commit. Is that the case? I've
> always thought that a commit should really have a JIRA just for
> tracking, but I can change that thinking. Can you elaborate on how you
> see JIRA playing into CTR?
Before we had JIRA at the ASF (only bugzilla, which is no fun at all
IMHO), we did CTR without filing issues, of course.
Sometimes, patches where attached on-list.
Today, most projects indeed file a JIRA for everything, to benefit
from fully JIRA-generated release notes etc.
CTR works great with JIRA:
+ Create JIRA issue
+ commit a patch, using the JIRA name ("CHUKWA-1234") in the commit log message
+ JIRA automatically detects and attaches the svn change log to the
issue (see the "subversion commits" tab in JIRA)
(+ iterate with more commits)
+ Close the issue (, reopen etc.)
> - Similarly, it sounds like patches aren't needed with CTR. The
> downside of this is that others can't easily apply the patch to a
> release distribution and rebuild (something I do all the time). I'd
> think if it's worth committing, it's worth having a patch file for
> others to apply. Any thoughts around this?
The svn commit log and the chukwa-commit mailing list give you an easy
way to access patches.
> I generally like the discussion around how to become more agile
> though, since I see many benefits to be had there. I recently skimmed
> some of the CTR discussion on the apache site and thought to myself
> that that sounds different than what we're doing.
Well, with this discussion thread I just meant to spin CTR. If RTC (or
any relaxed derivative thereof) is the way *you* as committers want to
go ahead, I definitively support it.
I don't mean to discrupt any well-established process just for the
sake of disruption.
Bernd
> On Mon, Sep 13, 2010 at 1:14 PM, Ariel Rabkin <as...@gmail.com> wrote:
>>
>> Our methodology is actually some hybrid; something like "wait for
>> potential review", then commit. Usually I'll commit after a few days
>> if nobody objected or commented. (I try to say explicitly when I'll
>> time out waiting for comments.) And most of the patch review is
>> fairly cursory.
>>
>> I agree that the full rigor of RTC is probably unnecessary. I do like
>> it, however, for bigger changes, particularly those that alter the
>> architecture or user visible feature set.
>>
>> Is it reasonable to say "for bugs, CTR is fine, and only file a JIRA
>> and wait for review if it's going to break something"?
>>
>> --Ari
>>
>> On Mon, Sep 13, 2010 at 12:58 PM, Bernd Fondermann
>> <be...@googlemail.com> wrote:
>> > Hi,
>> >
>> > If I understand correctly, Chukwa is following the review-then-commit
>> > (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
>> > only on positive feedback it is committed.
>> > As far as I can see, this is inherited from Hadoop's policies.
>> > However, most projects at the ASF apply commit-then-review (CTR). CTR
>> > has the advantage of being more agile, requiring less work (creating
>> > issue, patch file, attaching it, waiting for feedback etc.) while
>> > providing full oversight:
>> > Every commit is reviewed by other committers after it happened, can be
>> > discussed, reverted, improved etc. as a 'work in progress'.
>> > It is best practice in CTR-mode to selectively use RTC, e.g. for big
>> > patches or for potentially delicate commits.
>> >
>> > I think Chukwa would profit from changing to CTR, so I'd like to know
>> > what you think about it.
>> >
>> > Thanks,
>> >
>> > Bernd
>> >
>>
>>
>>
>> --
>> Ari Rabkin asrabkin@gmail.com
>> UC Berkeley Computer Science Department
>
Re: [DISCUSS] Applying commit-then-review
Posted by Bill Graham <bi...@gmail.com>.
I can see advantages to both CTR and RTC. For bugs and even modest
changes, yes CTR seems fine. Recent examples where I would have
preferred to do that include:
https://issues.apache.org/jira/browse/CHUKWA-517
https://issues.apache.org/jira/browse/CHUKWA-518
https://issues.apache.org/jira/browse/CHUKWA-519
Bigger changes though, especially ones that have an external face on
them, like the recent REST API features, are good to have more of a
feedback gathering RTC phase, like these:
https://issues.apache.org/jira/browse/CHUKWA-515
https://issues.apache.org/jira/browse/CHUKWA-520
A few questions a have though about CTR:
- Bernd, from your initial comments it sounds like in some cases you
don't even have a JIRA filed with the commit. Is that the case? I've
always thought that a commit should really have a JIRA just for
tracking, but I can change that thinking. Can you elaborate on how you
see JIRA playing into CTR?
- Similarly, it sounds like patches aren't needed with CTR. The
downside of this is that others can't easily apply the patch to a
release distribution and rebuild (something I do all the time). I'd
think if it's worth committing, it's worth having a patch file for
others to apply. Any thoughts around this?
I generally like the discussion around how to become more agile
though, since I see many benefits to be had there. I recently skimmed
some of the CTR discussion on the apache site and thought to myself
that that sounds different than what we're doing.
On Mon, Sep 13, 2010 at 1:14 PM, Ariel Rabkin <as...@gmail.com> wrote:
>
> Our methodology is actually some hybrid; something like "wait for
> potential review", then commit. Usually I'll commit after a few days
> if nobody objected or commented. (I try to say explicitly when I'll
> time out waiting for comments.) And most of the patch review is
> fairly cursory.
>
> I agree that the full rigor of RTC is probably unnecessary. I do like
> it, however, for bigger changes, particularly those that alter the
> architecture or user visible feature set.
>
> Is it reasonable to say "for bugs, CTR is fine, and only file a JIRA
> and wait for review if it's going to break something"?
>
> --Ari
>
> On Mon, Sep 13, 2010 at 12:58 PM, Bernd Fondermann
> <be...@googlemail.com> wrote:
> > Hi,
> >
> > If I understand correctly, Chukwa is following the review-then-commit
> > (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
> > only on positive feedback it is committed.
> > As far as I can see, this is inherited from Hadoop's policies.
> > However, most projects at the ASF apply commit-then-review (CTR). CTR
> > has the advantage of being more agile, requiring less work (creating
> > issue, patch file, attaching it, waiting for feedback etc.) while
> > providing full oversight:
> > Every commit is reviewed by other committers after it happened, can be
> > discussed, reverted, improved etc. as a 'work in progress'.
> > It is best practice in CTR-mode to selectively use RTC, e.g. for big
> > patches or for potentially delicate commits.
> >
> > I think Chukwa would profit from changing to CTR, so I'd like to know
> > what you think about it.
> >
> > Thanks,
> >
> > Bernd
> >
>
>
>
> --
> Ari Rabkin asrabkin@gmail.com
> UC Berkeley Computer Science Department
Re: [DISCUSS] Applying commit-then-review
Posted by Eric Yang <ey...@yahoo-inc.com>.
I like the current model where commit is allowed for silent consent. I usually leave my patches open for review for extensive period of time, and commit if no response. For large change, I do follow review then commit to ensure that I don't break other people's usage pattern. This has worked for Chukwa because there is no full time developer. I think it is a value added if there are full time developer that like to be more agile. I am open to CTR.
Regards,
Eric
On 9/13/10 1:14 PM, "Ariel Rabkin" <as...@gmail.com> wrote:
Our methodology is actually some hybrid; something like "wait for
potential review", then commit. Usually I'll commit after a few days
if nobody objected or commented. (I try to say explicitly when I'll
time out waiting for comments.) And most of the patch review is
fairly cursory.
I agree that the full rigor of RTC is probably unnecessary. I do like
it, however, for bigger changes, particularly those that alter the
architecture or user visible feature set.
Is it reasonable to say "for bugs, CTR is fine, and only file a JIRA
and wait for review if it's going to break something"?
--Ari
On Mon, Sep 13, 2010 at 12:58 PM, Bernd Fondermann
<be...@googlemail.com> wrote:
> Hi,
>
> If I understand correctly, Chukwa is following the review-then-commit
> (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
> only on positive feedback it is committed.
> As far as I can see, this is inherited from Hadoop's policies.
> However, most projects at the ASF apply commit-then-review (CTR). CTR
> has the advantage of being more agile, requiring less work (creating
> issue, patch file, attaching it, waiting for feedback etc.) while
> providing full oversight:
> Every commit is reviewed by other committers after it happened, can be
> discussed, reverted, improved etc. as a 'work in progress'.
> It is best practice in CTR-mode to selectively use RTC, e.g. for big
> patches or for potentially delicate commits.
>
> I think Chukwa would profit from changing to CTR, so I'd like to know
> what you think about it.
>
> Thanks,
>
> Bernd
>
--
Ari Rabkin asrabkin@gmail.com
UC Berkeley Computer Science Department
Re: [DISCUSS] Applying commit-then-review
Posted by Ariel Rabkin <as...@gmail.com>.
Our methodology is actually some hybrid; something like "wait for
potential review", then commit. Usually I'll commit after a few days
if nobody objected or commented. (I try to say explicitly when I'll
time out waiting for comments.) And most of the patch review is
fairly cursory.
I agree that the full rigor of RTC is probably unnecessary. I do like
it, however, for bigger changes, particularly those that alter the
architecture or user visible feature set.
Is it reasonable to say "for bugs, CTR is fine, and only file a JIRA
and wait for review if it's going to break something"?
--Ari
On Mon, Sep 13, 2010 at 12:58 PM, Bernd Fondermann
<be...@googlemail.com> wrote:
> Hi,
>
> If I understand correctly, Chukwa is following the review-then-commit
> (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
> only on positive feedback it is committed.
> As far as I can see, this is inherited from Hadoop's policies.
> However, most projects at the ASF apply commit-then-review (CTR). CTR
> has the advantage of being more agile, requiring less work (creating
> issue, patch file, attaching it, waiting for feedback etc.) while
> providing full oversight:
> Every commit is reviewed by other committers after it happened, can be
> discussed, reverted, improved etc. as a 'work in progress'.
> It is best practice in CTR-mode to selectively use RTC, e.g. for big
> patches or for potentially delicate commits.
>
> I think Chukwa would profit from changing to CTR, so I'd like to know
> what you think about it.
>
> Thanks,
>
> Bernd
>
--
Ari Rabkin asrabkin@gmail.com
UC Berkeley Computer Science Department
Re: [DISCUSS] Applying commit-then-review
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 9/21/2010 3:50 PM, Ariel Rabkin wrote:
> My sense is that there's virtually never feedback on patches prior to commit.
> There is, however, often feedback on design choices before the patch is posted.
>
> I assume even with commit-then-review there would still be review for
> major change proposals before implementation.
That's what we've done at httpd. Often a sandbox is created in svn to help
facilitate collaboration on a particular implementation experiment, and then
merged back to trunk once there is consensus around the major new idea.
Re: [DISCUSS] Applying commit-then-review
Posted by Ariel Rabkin <as...@gmail.com>.
My sense is that there's virtually never feedback on patches prior to commit.
There is, however, often feedback on design choices before the patch is posted.
I assume even with commit-then-review there would still be review for
major change proposals before implementation.
--Ari
On Mon, Sep 20, 2010 at 10:55 PM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 9/13/2010 2:58 PM, Bernd Fondermann wrote:
>> Hi,
>>
>> If I understand correctly, Chukwa is following the review-then-commit
>> (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
>> only on positive feedback it is committed.
>> As far as I can see, this is inherited from Hadoop's policies.
>> However, most projects at the ASF apply commit-then-review (CTR). CTR
>> has the advantage of being more agile, requiring less work (creating
>> issue, patch file, attaching it, waiting for feedback etc.) while
>> providing full oversight:
>> Every commit is reviewed by other committers after it happened, can be
>> discussed, reverted, improved etc. as a 'work in progress'.
>> It is best practice in CTR-mode to selectively use RTC, e.g. for big
>> patches or for potentially delicate commits.
>>
>> I think Chukwa would profit from changing to CTR, so I'd like to know
>> what you think about it.
>
> The only useful question is what % of the jira tickets are rejected, or
> corrected, prior to commit? If this number is very low, I'd suggest that
> waiting for jira feedback before committing is a waste. As this number
> grows larger, the amount of work undone in trunk becomes more onerous
> than the review of open jira tickets.
>
--
Ari Rabkin asrabkin@gmail.com
UC Berkeley Computer Science Department
Re: [DISCUSS] Applying commit-then-review
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 9/13/2010 2:58 PM, Bernd Fondermann wrote:
> Hi,
>
> If I understand correctly, Chukwa is following the review-then-commit
> (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
> only on positive feedback it is committed.
> As far as I can see, this is inherited from Hadoop's policies.
> However, most projects at the ASF apply commit-then-review (CTR). CTR
> has the advantage of being more agile, requiring less work (creating
> issue, patch file, attaching it, waiting for feedback etc.) while
> providing full oversight:
> Every commit is reviewed by other committers after it happened, can be
> discussed, reverted, improved etc. as a 'work in progress'.
> It is best practice in CTR-mode to selectively use RTC, e.g. for big
> patches or for potentially delicate commits.
>
> I think Chukwa would profit from changing to CTR, so I'd like to know
> what you think about it.
The only useful question is what % of the jira tickets are rejected, or
corrected, prior to commit? If this number is very low, I'd suggest that
waiting for jira feedback before committing is a waste. As this number
grows larger, the amount of work undone in trunk becomes more onerous
than the review of open jira tickets.
Re: [DISCUSS] Applying commit-then-review
Posted by Jiaqi Tan <ta...@gmail.com>.
I second the RTC model for Chukwa; I think the group of committers is
small enough that RTC will still be quite agile.
Jiaqi
On Tue, Sep 21, 2010 at 1:00 PM, Jerome Boulon <jb...@netflix.com> wrote:
> I would like to keep the review-then-commit (RTC) pattern.
> This should theoretically prevent anyone from adding non-tested/non-valid code and since we have to pass the review step the code should be better.
> Also, theoretically this should gave a trunk that is almost always good.
> /Jerome.
>
>
> On 9/13/10 12:58 PM, "Bernd Fondermann" <be...@googlemail.com> wrote:
>
> Hi,
>
> If I understand correctly, Chukwa is following the review-then-commit
> (RTC) pattern: Before every commit, a patch gets posted to a JIRA and
> only on positive feedback it is committed.
> As far as I can see, this is inherited from Hadoop's policies.
> However, most projects at the ASF apply commit-then-review (CTR). CTR
> has the advantage of being more agile, requiring less work (creating
> issue, patch file, attaching it, waiting for feedback etc.) while
> providing full oversight:
> Every commit is reviewed by other committers after it happened, can be
> discussed, reverted, improved etc. as a 'work in progress'.
> It is best practice in CTR-mode to selectively use RTC, e.g. for big
> patches or for potentially delicate commits.
>
> I think Chukwa would profit from changing to CTR, so I'd like to know
> what you think about it.
>
> Thanks,
>
> Bernd
>
>
>
Re: [DISCUSS] Applying commit-then-review
Posted by Jerome Boulon <jb...@netflix.com>.
I would like to keep the review-then-commit (RTC) pattern.
This should theoretically prevent anyone from adding non-tested/non-valid code and since we have to pass the review step the code should be better.
Also, theoretically this should gave a trunk that is almost always good.
/Jerome.
On 9/13/10 12:58 PM, "Bernd Fondermann" <be...@googlemail.com> wrote:
Hi,
If I understand correctly, Chukwa is following the review-then-commit
(RTC) pattern: Before every commit, a patch gets posted to a JIRA and
only on positive feedback it is committed.
As far as I can see, this is inherited from Hadoop's policies.
However, most projects at the ASF apply commit-then-review (CTR). CTR
has the advantage of being more agile, requiring less work (creating
issue, patch file, attaching it, waiting for feedback etc.) while
providing full oversight:
Every commit is reviewed by other committers after it happened, can be
discussed, reverted, improved etc. as a 'work in progress'.
It is best practice in CTR-mode to selectively use RTC, e.g. for big
patches or for potentially delicate commits.
I think Chukwa would profit from changing to CTR, so I'd like to know
what you think about it.
Thanks,
Bernd