[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r31506 - branches/1.5.x/subversion/svn

From: David Glasser <glasser_at_davidglasser.net>
Date: Thu, 29 May 2008 11:36:20 -0700

On Thu, May 29, 2008 at 5:29 AM, Jens Seidel <jensseidel_at_users.sf.net> wrote:
> On Wed, May 28, 2008 at 11:22:13PM +0200, Lieven Govaerts wrote:
>> jensseidel_at_tigris.org wrote:
>> > Author: jensseidel
>> > Date: Wed May 28 13:24:10 2008
>> > New Revision: 31506
>> >
>> > Log:
>> > On the '1.5.x' branch:
>> > Follow-up to r31501:
>> >
>> > * subversion/svn/main.c
>> > (svn_cl__options."accept"): Added a missing comma.
>> > This also brings some translations back to 100%.
>> >
>> > Modified:
>> > branches/1.5.x/subversion/svn/main.c
>> >
>> > Modified: branches/1.5.x/subversion/svn/main.c
>> > URL: http://svn.collab.net/viewvc/svn/branches/1.5.x/subversion/svn/main.c?pathrev=31506&r1=31505&r2=31506
>> > ==============================================================================
>> > --- branches/1.5.x/subversion/svn/main.c Wed May 28 13:05:49 2008 (r31505)
>> > +++ branches/1.5.x/subversion/svn/main.c Wed May 28 13:24:10 2008 (r31506)
>> > @@ -259,7 +259,7 @@ const apr_getopt_option_t svn_cl__option
>> > {"accept", opt_accept, 1,
>> > N_("specify automatic conflict resolution action\n"
>> > " "
>> > - "('postpone', 'base', 'mine-full', 'theirs-full'\n"
>> > + "('postpone', 'base', 'mine-full', 'theirs-full',\n"
>> > " "
>> > " 'edit', 'launch')")},
>> > /* 'mine-conflict' and 'theirs-conflict' are not
>> >
>> >
>> As simple as this change might be, it breaks one of the getopt tests. The getopt tests are one of the only tests really matching the actual string content character per character. I know these tests are overly strict, but while we have them I propose you run at least those before you commit a text change inside the code.
>
> No, I don't think I made the test break, I fixed it again! Please see r31501:
>
> ...
> - " '" SVN_CL__ACCEPT_THEIRS_FULL "',"
> + "('postpone', 'base', 'mine-full', 'theirs-full'\n"
>
> Do you see that the comma is missing in this patch? This made one of the
> messages fuzzy and a few translations were no longer up-to-date which I
> updated manually before.
>
> So to summarize: I just fixed it instead of starting changing main.c in
> 1.5.x. You may have noticed that I properly worked around this problem
> in trunk, in 1.5.x I only touched PO files except this one!

While your change looks good, and now that it's been reviewed is
welcome, even full committers shouldn't be committing changes to core
code in a release branch (especially a release branch for the biggest
post-1.0 release which is due to be rolled in a matter of days)
without some discussion; generally in STATUS but if you really think
it's obvious-fix enough, on dev@ or #svn-dev.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-29 20:36:30 CEST

This is an archived mail posted to the Subversion Dev mailing list.