Re: [PATCH] Fix to svn_time_to_human_cstring() - take 3

Ben Reser <ben@reser.org> writes:
> > > + }
> > > + else
> > > + apr_cpystrn(curptr, utf8_string, SVN_TIME__MAX_LENGTH - len);
> > > + }
> > > +
> > > return datestr;
> > > }
>
> The else statement still isn't compliant with the style we follow.
>
> if (err)
> {
> ..
> }
> else
> ..
>
> Please refer to HACKING and
> http://www.gnu.org/prep/standards_23.html#SEC23

Aside from this minor nit, and the later one about space-before-paren,
are there any substantive problems with this patch?

(How is his 'else' statement not compliant, by the way? I might be
missing something obvious, but I don't see it.)

Anyway, my point is: when the only problems with a patch are of this
sort, we generally apply it (the committer fixes the formatting) and
just mention the tweaks when we respond saying its been applied.

Sure, the goal is for people to eventually learn the formatting
conventions. But we don't have to do that by iterating the same patch
down to the Nth degree of formatting perfection. It's more productive
& happier for everyone if those next iterations can happen on a *new*
patch, so Subversion gets better faster :-).

Let's not make people jump through too many hoops here...

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 6 23:07:57 2004