You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/03/01 00:13:39 UTC

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

You should let the RM decide what is "obvious" or not, what should be
merged, and to do the actual merging.

On Sat, Feb 28, 2009 at 22:28, Arfrever Frehtes Taifersar Arahesis
<Ar...@gmail.com> wrote:
> Author: arfrever
> Date: Sat Feb 28 13:28:46 2009
> New Revision: 36214
>
> Log:
> On the '1.6.x' branch:
> Merge obvious fix r36213 from trunk.
>
>  * r36213
>   * subversion/svn/main.c
>     (svn_cl__cmd_table."status"): Update help message for 'svn status'.
>
> Modified:
>   branches/1.6.x/   (props changed)
>   branches/1.6.x/CHANGES   (props changed)
>   branches/1.6.x/subversion/svn/main.c
>
> Merged:
>   /trunk:r36213
>
> Modified: branches/1.6.x/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/branches/1.6.x/subversion/svn/main.c?pathrev=36214&r1=36213&r2=36214
> ==============================================================================
> --- branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:14:42 2009        (r36213)
> +++ branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:28:46 2009        (r36214)
> @@ -2,7 +2,7 @@
>  * main.c:  Subversion command line client.
>  *
>  * ====================================================================
> - * Copyright (c) 2000-2008 CollabNet.  All rights reserved.
> + * Copyright (c) 2000-2009 CollabNet.  All rights reserved.
>  *
>  * This software is licensed as described in the file COPYING, which
>  * you should have received as part of this distribution.  The terms
> @@ -899,7 +899,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
>      "    svn status\n"
>      "     M      wc/bar.c\n"
>      "    !     C wc/qaz.c\n"
> -     "          >   incoming edit, local missing\n"
> +     "          >   local missing, incoming edit upon update\n"
>      "    D       wc/qax.c\n"),
>     { 'u', 'v', 'N', opt_depth, 'q', opt_no_ignore, opt_incremental, opt_xml,
>       opt_ignore_externals, opt_changelist} },
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1246649
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1247503


Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 5, 2009 at 20:10, Stefan Sperling <st...@elego.de> wrote:
> On Mon, Mar 02, 2009 at 11:18:11AM +0000, Stefan Sperling wrote:
>> On Sun, Mar 01, 2009 at 08:15:59AM -0800, Justin Erenkrantz wrote:
>> > On Sun, Mar 1, 2009 at 8:08 AM, Greg Stein <gs...@gmail.com> wrote:
>> > > The release branch is about *stabilization*, and that process is under
>> > > the sole guidance of the Release Manager. That is *why* we have a
>> > > release manager. We cannot allow people to simply commit whatever they
>> > > feel is "obvious" to a branch that we are stabilizing for release. If
>> > > it is obvious, then it will get voted on and merged.
>> >
>> > +1.
>> >
>> > One person's obvious fix is another person's "wait a sec, I need to
>> > review" patch.  -- justin
>>
>> Fair enough. If a fix is obvious, it's more than likely going to
>> gather enough votes quickly anyway.
>
> Right guys, the thing is in STATUS now as per your request, so please vote
> on it if you have the time.
>
> It's an obvious fix and I'd like to get it into rc4.

IMO, it is *not* an obvious fix. In order to verify its correctness, I
would have to get a WC into a tree-conflicted state and then run 'svn
status' to verify the output.

Obvious fixes are typos, docstring updates, whitespace changes, or
other tweaks that don't affect the code or its output.

Look through the logs for other cases of "obvious fixes". They are
quite a bit different.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1273625


Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Mar 02, 2009 at 11:18:11AM +0000, Stefan Sperling wrote:
> On Sun, Mar 01, 2009 at 08:15:59AM -0800, Justin Erenkrantz wrote:
> > On Sun, Mar 1, 2009 at 8:08 AM, Greg Stein <gs...@gmail.com> wrote:
> > > The release branch is about *stabilization*, and that process is under
> > > the sole guidance of the Release Manager. That is *why* we have a
> > > release manager. We cannot allow people to simply commit whatever they
> > > feel is "obvious" to a branch that we are stabilizing for release. If
> > > it is obvious, then it will get voted on and merged.
> > 
> > +1.
> > 
> > One person's obvious fix is another person's "wait a sec, I need to
> > review" patch.  -- justin
> 
> Fair enough. If a fix is obvious, it's more than likely going to
> gather enough votes quickly anyway.

Right guys, the thing is in STATUS now as per your request, so please vote
on it if you have the time.

It's an obvious fix and I'd like to get it into rc4.

Stefan

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Mar 01, 2009 at 08:15:59AM -0800, Justin Erenkrantz wrote:
> On Sun, Mar 1, 2009 at 8:08 AM, Greg Stein <gs...@gmail.com> wrote:
> > The release branch is about *stabilization*, and that process is under
> > the sole guidance of the Release Manager. That is *why* we have a
> > release manager. We cannot allow people to simply commit whatever they
> > feel is "obvious" to a branch that we are stabilizing for release. If
> > it is obvious, then it will get voted on and merged.
> 
> +1.
> 
> One person's obvious fix is another person's "wait a sec, I need to
> review" patch.  -- justin

Fair enough. If a fix is obvious, it's more than likely going to
gather enough votes quickly anyway.

Stefan

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Justin Erenkrantz <je...@apache.org>.
On Sun, Mar 1, 2009 at 8:08 AM, Greg Stein <gs...@gmail.com> wrote:
> The release branch is about *stabilization*, and that process is under
> the sole guidance of the Release Manager. That is *why* we have a
> release manager. We cannot allow people to simply commit whatever they
> feel is "obvious" to a branch that we are stabilizing for release. If
> it is obvious, then it will get voted on and merged.

+1.

One person's obvious fix is another person's "wait a sec, I need to
review" patch.  -- justin

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1251099

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Mar 1, 2009, at 10:08 AM, Greg Stein wrote:

> On Sun, Mar 1, 2009 at 16:25, Stefan Sperling <st...@elego.de> wrote:
>> On Sun, Mar 01, 2009 at 01:49:48AM +0100, Greg Stein wrote:
>>> That applies to partial committers fixing "obvious" things outside
>>> their normal allowed area. It does not apply to merging changes  
>>> onto a
>>> release branch. The description of change management for release
>>> branches even refers to obvious fixes when talking about *voting*  
>>> for
>>> those types of changes.
>>
>> Where?
>
> #obvious-fixes is the section that covers "Obvious fixes" and it is
> only in reference to partial committers making "obvious fixes" outside
> of their normal area.
>
>> All I could find is that "Obvious fixes do not require '(rX only)'
>> to be mentioned".
>
> "... to be mentioned [IN THE VOTING]." It says nothing about "go ahead
> and commit whatever you think is obvious to the release branch."
>
>> I don't see a problem with invoking the obvious fix rule in this  
>> case.
>
> I do. Very much so.
>
> The release branch is about *stabilization*, and that process is under
> the sole guidance of the Release Manager. That is *why* we have a
> release manager. We cannot allow people to simply commit whatever they
> feel is "obvious" to a branch that we are stabilizing for release. If
> it is obvious, then it will get voted on and merged.
>
>> Arfrever mentioned that he invoked the rule, making clear to everyone
>> what his reasoning for merging the change is. So the only thing worth
>> arguing is whether we all agree the commit was an obvious fix or not.
>
> Right. And that is why we have VOTING. The release branch is *closed*
> to all but the Release Manager. We vote to get changes merged. We
> don't simply declare it "obvious" and merge it.
>
>>> And (IMO) merging changes into a release should be deferred to the  
>>> RM
>>> simply for politeness' sake.
>>
>> I have made merges into release branches before.
>> I'd say once there are enough votes, it does not matter who does the
>> merging. I see no issue of politeness.
>
> You're circumventing the Release Manager. I consider that impolite.
> You're basically saying, "I'm going to do this without your review,
> permission, or oversight. Your role does not matter to me."
>
> (note that Hyrum may not care, but I do; the RM's role is a social
> contract that is a Good Thing to honor)

Being the current RM, perhaps I should chime in here. :)

Back in the day, whoever approved the patch or patch group in STATUS  
usually did the merge.  It wasn't until I became the RM and started  
paying close attention to STATUS and applying my OCD to it did the  
Release Manager become the sole merger and guardian of the branch.  It  
wasn't a conscious decision on the part of the project, but rather  
something that just happened.

That being said, I think it's a Good Thing to follow this convention.   
Having a single person through whom all the patches go, makes sure  
that the RM is aware of what's in the branch and what's not and just  
the general state of things.  Note that he isn't a gate-keeper and  
doesn't decide what goes in and what doesn't (a la Linus), but rather  
he's the person who just does the actual merging.

(And lest people think that I'm on some kind of power trip, as I've  
looked at other project's release processes, this is the typical  
strategy in the vast majority of instances.  We're much more loose  
when it comes to this type of thing.)

Now, regarding Arfrever's specific change: I know he did it mainly for  
the translators, and in my anglo-centric world, I'm not particularly  
sensitive to those issues.  I've got no problem with the change going  
in, but I do think it's not an obvious fix and it should go through  
STATUS.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1256411

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Greg Stein <gs...@gmail.com>.
On Sun, Mar 1, 2009 at 16:25, Stefan Sperling <st...@elego.de> wrote:
> On Sun, Mar 01, 2009 at 01:49:48AM +0100, Greg Stein wrote:
>> That applies to partial committers fixing "obvious" things outside
>> their normal allowed area. It does not apply to merging changes onto a
>> release branch. The description of change management for release
>> branches even refers to obvious fixes when talking about *voting* for
>> those types of changes.
>
> Where?

#obvious-fixes is the section that covers "Obvious fixes" and it is
only in reference to partial committers making "obvious fixes" outside
of their normal area.

> All I could find is that "Obvious fixes do not require '(rX only)'
> to be mentioned".

"... to be mentioned [IN THE VOTING]." It says nothing about "go ahead
and commit whatever you think is obvious to the release branch."

> I don't see a problem with invoking the obvious fix rule in this case.

I do. Very much so.

The release branch is about *stabilization*, and that process is under
the sole guidance of the Release Manager. That is *why* we have a
release manager. We cannot allow people to simply commit whatever they
feel is "obvious" to a branch that we are stabilizing for release. If
it is obvious, then it will get voted on and merged.

> Arfrever mentioned that he invoked the rule, making clear to everyone
> what his reasoning for merging the change is. So the only thing worth
> arguing is whether we all agree the commit was an obvious fix or not.

Right. And that is why we have VOTING. The release branch is *closed*
to all but the Release Manager. We vote to get changes merged. We
don't simply declare it "obvious" and merge it.

>> And (IMO) merging changes into a release should be deferred to the RM
>> simply for politeness' sake.
>
> I have made merges into release branches before.
> I'd say once there are enough votes, it does not matter who does the
> merging. I see no issue of politeness.

You're circumventing the Release Manager. I consider that impolite.
You're basically saying, "I'm going to do this without your review,
permission, or oversight. Your role does not matter to me."

(note that Hyrum may not care, but I do; the RM's role is a social
contract that is a Good Thing to honor)

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1251060

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Mar 01, 2009 at 01:49:48AM +0100, Greg Stein wrote:
> That applies to partial committers fixing "obvious" things outside
> their normal allowed area. It does not apply to merging changes onto a
> release branch. The description of change management for release
> branches even refers to obvious fixes when talking about *voting* for
> those types of changes.

Where?

All I could find is that "Obvious fixes do not require '(rX only)'
to be mentioned".

I don't see a problem with invoking the obvious fix rule in this case.
Arfrever mentioned that he invoked the rule, making clear to everyone
what his reasoning for merging the change is. So the only thing worth
arguing is whether we all agree the commit was an obvious fix or not.

> And (IMO) merging changes into a release should be deferred to the RM
> simply for politeness' sake.

I have made merges into release branches before.
I'd say once there are enough votes, it does not matter who does the
merging. I see no issue of politeness.

Stefan

Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Greg Stein <gs...@gmail.com>.
That applies to partial committers fixing "obvious" things outside
their normal allowed area. It does not apply to merging changes onto a
release branch. The description of change management for release
branches even refers to obvious fixes when talking about *voting* for
those types of changes.

And (IMO) merging changes into a release should be deferred to the RM
simply for politeness' sake.

On Sun, Mar 1, 2009 at 01:23, Arfrever Frehtes Taifersar Arahesis
<ar...@gmail.com> wrote:
> 2009-03-01 01:13:39 Greg Stein napisał(a):
>> You should let the RM decide what is "obvious" or not, what should be
>> merged, and to do the actual merging.
>
> http://subversion.tigris.org/hacking.html#obvious-fix
>
>> On Sat, Feb 28, 2009 at 22:28, Arfrever Frehtes Taifersar Arahesis
>> <Ar...@gmail.com> wrote:
>> > Author: arfrever
>> > Date: Sat Feb 28 13:28:46 2009
>> > New Revision: 36214
>> >
>> > Log:
>> > On the '1.6.x' branch:
>> > Merge obvious fix r36213 from trunk.
>> >
>> >  * r36213
>> >   * subversion/svn/main.c
>> >     (svn_cl__cmd_table."status"): Update help message for 'svn status'.
>> >
>> > Modified:
>> >   branches/1.6.x/   (props changed)
>> >   branches/1.6.x/CHANGES   (props changed)
>> >   branches/1.6.x/subversion/svn/main.c
>> >
>> > Merged:
>> >   /trunk:r36213
>> >
>> > Modified: branches/1.6.x/subversion/svn/main.c
>> > URL: http://svn.collab.net/viewvc/svn/branches/1.6.x/subversion/svn/main.c?pathrev=36214&r1=36213&r2=36214
>> > ==============================================================================
>> > --- branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:14:42 2009        (r36213)
>> > +++ branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:28:46 2009        (r36214)
>> > @@ -2,7 +2,7 @@
>> >  * main.c:  Subversion command line client.
>> >  *
>> >  * ====================================================================
>> > - * Copyright (c) 2000-2008 CollabNet.  All rights reserved.
>> > + * Copyright (c) 2000-2009 CollabNet.  All rights reserved.
>> >  *
>> >  * This software is licensed as described in the file COPYING, which
>> >  * you should have received as part of this distribution.  The terms
>> > @@ -899,7 +899,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
>> >      "    svn status\n"
>> >      "     M      wc/bar.c\n"
>> >      "    !     C wc/qaz.c\n"
>> > -     "          >   incoming edit, local missing\n"
>> > +     "          >   local missing, incoming edit upon update\n"
>> >      "    D       wc/qax.c\n"),
>> >     { 'u', 'v', 'N', opt_depth, 'q', opt_no_ignore, opt_incremental, opt_xml,
>> >       opt_ignore_externals, opt_changelist} },
>
> --
> Arfrever Frehtes Taifersar Arahesis
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1247676


Re: svn commit: r36214 - in branches/1.6.x: . subversion/svn

Posted by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com>.
2009-03-01 01:13:39 Greg Stein napisał(a):
> You should let the RM decide what is "obvious" or not, what should be
> merged, and to do the actual merging.

http://subversion.tigris.org/hacking.html#obvious-fix

> On Sat, Feb 28, 2009 at 22:28, Arfrever Frehtes Taifersar Arahesis
> <Ar...@gmail.com> wrote:
> > Author: arfrever
> > Date: Sat Feb 28 13:28:46 2009
> > New Revision: 36214
> >
> > Log:
> > On the '1.6.x' branch:
> > Merge obvious fix r36213 from trunk.
> >
> >  * r36213
> >   * subversion/svn/main.c
> >     (svn_cl__cmd_table."status"): Update help message for 'svn status'.
> >
> > Modified:
> >   branches/1.6.x/   (props changed)
> >   branches/1.6.x/CHANGES   (props changed)
> >   branches/1.6.x/subversion/svn/main.c
> >
> > Merged:
> >   /trunk:r36213
> >
> > Modified: branches/1.6.x/subversion/svn/main.c
> > URL: http://svn.collab.net/viewvc/svn/branches/1.6.x/subversion/svn/main.c?pathrev=36214&r1=36213&r2=36214
> > ==============================================================================
> > --- branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:14:42 2009        (r36213)
> > +++ branches/1.6.x/subversion/svn/main.c        Sat Feb 28 13:28:46 2009        (r36214)
> > @@ -2,7 +2,7 @@
> >  * main.c:  Subversion command line client.
> >  *
> >  * ====================================================================
> > - * Copyright (c) 2000-2008 CollabNet.  All rights reserved.
> > + * Copyright (c) 2000-2009 CollabNet.  All rights reserved.
> >  *
> >  * This software is licensed as described in the file COPYING, which
> >  * you should have received as part of this distribution.  The terms
> > @@ -899,7 +899,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
> >      "    svn status\n"
> >      "     M      wc/bar.c\n"
> >      "    !     C wc/qaz.c\n"
> > -     "          >   incoming edit, local missing\n"
> > +     "          >   local missing, incoming edit upon update\n"
> >      "    D       wc/qax.c\n"),
> >     { 'u', 'v', 'N', opt_depth, 'q', opt_no_ignore, opt_incremental, opt_xml,
> >       opt_ignore_externals, opt_changelist} },

-- 
Arfrever Frehtes Taifersar Arahesis