You are viewing a plain text version of this content. The canonical link for it is here.
Posted to xbean-dev@geronimo.apache.org by Dain Sundstrom <da...@iq80.com> on 2006/08/23 21:01:02 UTC

Reviewing and committing

Geronimo is considering a change to its review and commit policies.   
As a subproject, I think we should discuss how we would like to  
handle reviewing code and when it should be committed. So...

How do you think we should handle this process?

What rules-of-thumb should we use to guide ourselves?

-dain

Re: Reviewing and committing

Posted by Alan Cabrera <li...@toolazydogs.com>.
Dain Sundstrom wrote:
> On Aug 23, 2006, at 5:24 PM, Guillaume Nodet wrote:
>
>> Is a subproject entitled to make such decisions ?
>
> Sorry, I didn't mean to imply we were able to make such decisions, but 
> we can make a recommendation to the PMC that they allow xbean to 
> operate under a different set of policies.
I like RTC.


Regards,
Alan




Re: Reviewing and committing

Posted by Dain Sundstrom <da...@iq80.com>.
On Aug 23, 2006, at 5:24 PM, Guillaume Nodet wrote:

> Is a subproject entitled to make such decisions ?

Sorry, I didn't mean to imply we were able to make such decisions,  
but we can make a recommendation to the PMC that they allow xbean to  
operate under a different set of policies.

-dain



Re: Reviewing and committing

Posted by Guillaume Nodet <gn...@gmail.com>.
Is a subproject entitled to make such decisions ?

Anyway, I'm all in favor for a relaxed RTC, i.e. one
should send a mail to the dev list annoucing what he is doing
when working on big changes / new features.
Lazy consensus seems the right thing for xbean to me.

For example, I guess that my last mail to this list was lost, or that
nobody had any strong objections about using qdox 1.6
which has been compiled with jdk 5.  This would make xbean-spring
tied to jdk 5 :(

On 8/23/06, Dain Sundstrom <da...@iq80.com> wrote:
>
> Geronimo is considering a change to its review and commit policies.
> As a subproject, I think we should discuss how we would like to
> handle reviewing code and when it should be committed. So...
>
> How do you think we should handle this process?
>
> What rules-of-thumb should we use to guide ourselves?
>
> -dain
>



-- 
Cheers,
Guillaume Nodet

Re: Reviewing and committing

Posted by David Blevins <da...@visi.com>.
On Aug 24, 2006, at 1:34 AM, James Strachan wrote:

> BTW how about a new model. Lets call it JCTR.
>
> * create a JIRA
> * commit patch and refer to the JIRA issue in the commit comment (**)
> * close the JIRA
> * other folks review, if need be we can reopen the JIRA again, revert
> the patch if need be or continue amending it etc
>
> (**) we can get JIRA to then link to the SVN revision so that folks
> can view the patch from JIRA so its similar to attaching a patch to
> JIRA just a whole lot less work)
>
> For really complex things we could still switch to full RTC if need be
> since the process is JIRA focussed.
>
> The basic idea is that all non-trivial changes should have a JIRA
> associated with them, if nothing else than to create good release
> notes - and we should endevour to have the SVN and JIRA's wired
> together better then folks can then track things via JIRA, email or
> SVN with them all linked together nicely.

This is my thinking for Geronimo as well, though I don't know if it's  
coming out real clear.  The RTC system is already in Jira with fancy- 
schmancy commit emails and everything, so if someone *wants* to use  
it, why not?  I.e "at your discretion."

Over time, we'd likely get some kind of rhythm going as to where it's  
effective and where it makes no difference and to here it's just a  
bad idea.

But the Jira thing is a given... as you say, release notes.  I do my  
best to file a Jira for anything the user will want to know about,  
even if I've already done the work.

-David


>
> On 8/24/06, James Strachan <ja...@gmail.com> wrote:
>> I'm a CTR fan - every project I've ever used and have ever worked on
>> works like that (apart from Geronimo) and its very effective, works
>> well, allows projects to be productive and usually changes are pretty
>> small so by reading the commit log/mails you can keep up to date. The
>> beauty of using a SCM is you can always roll stuff back if you need
>> to, if folks have an issue with a particular direction or change :).
>>
>> So in summary I'm agreeing with Guillaume - lets use CTR unless folks
>> think there is a particular area which requires RTC for big  
>> changes or
>> contentious issues etc.
>>
>>
>> On 8/23/06, Dain Sundstrom <da...@iq80.com> wrote:
>> > Geronimo is considering a change to its review and commit policies.
>> > As a subproject, I think we should discuss how we would like to
>> > handle reviewing code and when it should be committed. So...
>> >
>> > How do you think we should handle this process?
>> >
>> > What rules-of-thumb should we use to guide ourselves?
>> >
>> > -dain
>> >
>>
>>
>> --
>>
>> James
>> -------
>> http://radio.weblogs.com/0112098/
>>
>
>
> -- 
>
> James
> -------
> http://radio.weblogs.com/0112098/
>


Re: Reviewing and committing

Posted by James Strachan <ja...@gmail.com>.
BTW how about a new model. Lets call it JCTR.

* create a JIRA
* commit patch and refer to the JIRA issue in the commit comment (**)
* close the JIRA
* other folks review, if need be we can reopen the JIRA again, revert
the patch if need be or continue amending it etc

(**) we can get JIRA to then link to the SVN revision so that folks
can view the patch from JIRA so its similar to attaching a patch to
JIRA just a whole lot less work)

For really complex things we could still switch to full RTC if need be
since the process is JIRA focussed.

The basic idea is that all non-trivial changes should have a JIRA
associated with them, if nothing else than to create good release
notes - and we should endevour to have the SVN and JIRA's wired
together better then folks can then track things via JIRA, email or
SVN with them all linked together nicely.



On 8/24/06, James Strachan <ja...@gmail.com> wrote:
> I'm a CTR fan - every project I've ever used and have ever worked on
> works like that (apart from Geronimo) and its very effective, works
> well, allows projects to be productive and usually changes are pretty
> small so by reading the commit log/mails you can keep up to date. The
> beauty of using a SCM is you can always roll stuff back if you need
> to, if folks have an issue with a particular direction or change :).
>
> So in summary I'm agreeing with Guillaume - lets use CTR unless folks
> think there is a particular area which requires RTC for big changes or
> contentious issues etc.
>
>
> On 8/23/06, Dain Sundstrom <da...@iq80.com> wrote:
> > Geronimo is considering a change to its review and commit policies.
> > As a subproject, I think we should discuss how we would like to
> > handle reviewing code and when it should be committed. So...
> >
> > How do you think we should handle this process?
> >
> > What rules-of-thumb should we use to guide ourselves?
> >
> > -dain
> >
>
>
> --
>
> James
> -------
> http://radio.weblogs.com/0112098/
>


-- 

James
-------
http://radio.weblogs.com/0112098/

Re: Reviewing and committing

Posted by David Blevins <da...@visi.com>.
On Aug 25, 2006, at 12:48 PM, Guillaume Nodet wrote:

> On 8/24/06, Dain Sundstrom <da...@iq80.com> wrote:
>>
>> I think David's comments on geronimo dev are spot on.
>>
>> Begin forwarded message:
>>
>> > More thoughts on the "where" and "how" topic.
>> >
>> > So far my thoughts on "how"; review to your satisfaction and +1, 72
>> > hour cut off.
>> >
>> > As far as "where" ....
>> >
>> > I'm inclined to say "at your discretion" where the following are
>> > encouraged:
>> >  - Significant new functionality
>> >  - Significant changes
>> >  - Patches from Contributors
>> >  - Borderline "fixes" to a stable branch
>> >
>> > Whether or not it merits RTC would be at your discretion.  It is to
>> > your advantage in these situations because:
>> >
>> > - "Significant new functionality" and "Significant changes": It's a
>> >    "Get out of jail free" card.  Having more people understand your
>> >    code keeps you from spending all day on the user list.  You do
>> >    support your code on the user list, right?
>> >
>> > - "Patches from Contributors": Getting three votes for your patches
>> >    is not a bad way to, in time, get your three votes to be a
>> >    committer.  Let's be clear, someone who commits all your patches
>> >    with no review from others on the project isn't doing you any
>> >    favors.  It's in your interest to push to get your votes on  
>> every
>> >    patch.
>> >
>> > - "Borderline 'fixes' to a stable branch": It's a given you will
>> >    think everything you want to put in a stable branch is  
>> important.
>> >    But, is it a fix or is it a new feature?  If you think others  
>> may
>> >    disagree, you may want to put it up for review or you may find
>> >    yourself running the TCK all alone with no help.
>> >
>> >
>> > Those are the advantages you stand to gain should you choose to use
>> > RTC for any of the above situations.  RTC is not the only way to
>> > get the above benefits, so it is at your discretion whether or not
>> > your situation merits it.
>>
>> The only think I would change is the "how" section at the top.  I
>> propose we follow this process:
>>
>> To commit you need either 3 +1 (no -1s) from a committer or 72 hours
>> pass which ever happens first.  I suggest you complain loudly if you
>> get no comments after 48 hours.  As above a +1 means you have
>> "reviewed to you
>>
>> -dain
>>
>>
>>
> I agree with these guidelines.  Though, I would add that if there  
> is an
> ongoing
> discussion about the patch / feature, the discussion should be settled
> before
> being able to commit without having to cast a -1 (even if the 72 hours
> period
> is off).

Definitely.

-David

>
> -- 
> Cheers,
> Guillaume Nodet


Re: Reviewing and committing

Posted by Guillaume Nodet <gn...@gmail.com>.
On 8/24/06, Dain Sundstrom <da...@iq80.com> wrote:
>
> I think David's comments on geronimo dev are spot on.
>
> Begin forwarded message:
>
> > More thoughts on the "where" and "how" topic.
> >
> > So far my thoughts on "how"; review to your satisfaction and +1, 72
> > hour cut off.
> >
> > As far as "where" ....
> >
> > I'm inclined to say "at your discretion" where the following are
> > encouraged:
> >  - Significant new functionality
> >  - Significant changes
> >  - Patches from Contributors
> >  - Borderline "fixes" to a stable branch
> >
> > Whether or not it merits RTC would be at your discretion.  It is to
> > your advantage in these situations because:
> >
> > - "Significant new functionality" and "Significant changes": It's a
> >    "Get out of jail free" card.  Having more people understand your
> >    code keeps you from spending all day on the user list.  You do
> >    support your code on the user list, right?
> >
> > - "Patches from Contributors": Getting three votes for your patches
> >    is not a bad way to, in time, get your three votes to be a
> >    committer.  Let's be clear, someone who commits all your patches
> >    with no review from others on the project isn't doing you any
> >    favors.  It's in your interest to push to get your votes on every
> >    patch.
> >
> > - "Borderline 'fixes' to a stable branch": It's a given you will
> >    think everything you want to put in a stable branch is important.
> >    But, is it a fix or is it a new feature?  If you think others may
> >    disagree, you may want to put it up for review or you may find
> >    yourself running the TCK all alone with no help.
> >
> >
> > Those are the advantages you stand to gain should you choose to use
> > RTC for any of the above situations.  RTC is not the only way to
> > get the above benefits, so it is at your discretion whether or not
> > your situation merits it.
>
> The only think I would change is the "how" section at the top.  I
> propose we follow this process:
>
> To commit you need either 3 +1 (no -1s) from a committer or 72 hours
> pass which ever happens first.  I suggest you complain loudly if you
> get no comments after 48 hours.  As above a +1 means you have
> "reviewed to you
>
> -dain
>
>
>
I agree with these guidelines.  Though, I would add that if there is an
ongoing
discussion about the patch / feature, the discussion should be settled
before
being able to commit without having to cast a -1 (even if the 72 hours
period
is off).


-- 
Cheers,
Guillaume Nodet

Re: Reviewing and committing

Posted by Dain Sundstrom <da...@iq80.com>.
Any comments?  Should I let this go?

-dain

On Aug 24, 2006, at 9:45 AM, Dain Sundstrom wrote:

> I think David's comments on geronimo dev are spot on.
>
> Begin forwarded message:
>
>> More thoughts on the "where" and "how" topic.
>>
>> So far my thoughts on "how"; review to your satisfaction and +1,  
>> 72 hour cut off.
>>
>> As far as "where" ....
>>
>> I'm inclined to say "at your discretion" where the following are  
>> encouraged:
>>  - Significant new functionality
>>  - Significant changes
>>  - Patches from Contributors
>>  - Borderline "fixes" to a stable branch
>>
>> Whether or not it merits RTC would be at your discretion.  It is to
>> your advantage in these situations because:
>>
>> - "Significant new functionality" and "Significant changes": It's a
>>    "Get out of jail free" card.  Having more people understand your
>>    code keeps you from spending all day on the user list.  You do
>>    support your code on the user list, right?
>>
>> - "Patches from Contributors": Getting three votes for your patches
>>    is not a bad way to, in time, get your three votes to be a
>>    committer.  Let's be clear, someone who commits all your patches
>>    with no review from others on the project isn't doing you any
>>    favors.  It's in your interest to push to get your votes on every
>>    patch.
>>
>> - "Borderline 'fixes' to a stable branch": It's a given you will
>>    think everything you want to put in a stable branch is important.
>>    But, is it a fix or is it a new feature?  If you think others may
>>    disagree, you may want to put it up for review or you may find
>>    yourself running the TCK all alone with no help.
>>
>>
>> Those are the advantages you stand to gain should you choose to  
>> use RTC for any of the above situations.  RTC is not the only way  
>> to get the above benefits, so it is at your discretion whether or  
>> not your situation merits it.
>
> The only think I would change is the "how" section at the top.  I  
> propose we follow this process:
>
> To commit you need either 3 +1 (no -1s) from a committer or 72  
> hours pass which ever happens first.  I suggest you complain loudly  
> if you get no comments after 48 hours.  As above a +1 means you  
> have "reviewed to you
>
> -dain
>


Re: Reviewing and committing

Posted by Dain Sundstrom <da...@iq80.com>.
I think David's comments on geronimo dev are spot on.

Begin forwarded message:

> More thoughts on the "where" and "how" topic.
>
> So far my thoughts on "how"; review to your satisfaction and +1, 72  
> hour cut off.
>
> As far as "where" ....
>
> I'm inclined to say "at your discretion" where the following are  
> encouraged:
>  - Significant new functionality
>  - Significant changes
>  - Patches from Contributors
>  - Borderline "fixes" to a stable branch
>
> Whether or not it merits RTC would be at your discretion.  It is to
> your advantage in these situations because:
>
> - "Significant new functionality" and "Significant changes": It's a
>    "Get out of jail free" card.  Having more people understand your
>    code keeps you from spending all day on the user list.  You do
>    support your code on the user list, right?
>
> - "Patches from Contributors": Getting three votes for your patches
>    is not a bad way to, in time, get your three votes to be a
>    committer.  Let's be clear, someone who commits all your patches
>    with no review from others on the project isn't doing you any
>    favors.  It's in your interest to push to get your votes on every
>    patch.
>
> - "Borderline 'fixes' to a stable branch": It's a given you will
>    think everything you want to put in a stable branch is important.
>    But, is it a fix or is it a new feature?  If you think others may
>    disagree, you may want to put it up for review or you may find
>    yourself running the TCK all alone with no help.
>
>
> Those are the advantages you stand to gain should you choose to use  
> RTC for any of the above situations.  RTC is not the only way to  
> get the above benefits, so it is at your discretion whether or not  
> your situation merits it.

The only think I would change is the "how" section at the top.  I  
propose we follow this process:

To commit you need either 3 +1 (no -1s) from a committer or 72 hours  
pass which ever happens first.  I suggest you complain loudly if you  
get no comments after 48 hours.  As above a +1 means you have  
"reviewed to you

-dain



Re: Reviewing and committing

Posted by James Strachan <ja...@gmail.com>.
I'm a CTR fan - every project I've ever used and have ever worked on
works like that (apart from Geronimo) and its very effective, works
well, allows projects to be productive and usually changes are pretty
small so by reading the commit log/mails you can keep up to date. The
beauty of using a SCM is you can always roll stuff back if you need
to, if folks have an issue with a particular direction or change :).

So in summary I'm agreeing with Guillaume - lets use CTR unless folks
think there is a particular area which requires RTC for big changes or
contentious issues etc.


On 8/23/06, Dain Sundstrom <da...@iq80.com> wrote:
> Geronimo is considering a change to its review and commit policies.
> As a subproject, I think we should discuss how we would like to
> handle reviewing code and when it should be committed. So...
>
> How do you think we should handle this process?
>
> What rules-of-thumb should we use to guide ourselves?
>
> -dain
>


-- 

James
-------
http://radio.weblogs.com/0112098/