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

Re: [PATCH] allow svnsync to translate non-UTF-8 log messages to UTF-8 (v. 2)

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 21 Sep 2010 18:56:11 +0200

Daniel Trebbien wrote on Tue, Sep 21, 2010 at 08:12:38 -0700:
> On Sun, Sep 19, 2010 at 9:39 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> > Need the word "to" after the semicolon for the English to be correct.
>
> Fixed. (When I say "fixed", I mean that I committed a fix in my own
> fork of Subversion: http://github.com/dtrebbien/subversion )
>

Fine, but please post patches and log messages to this list as usual.
It's easier for me/us to process them that way. Thanks.

> >> +svn_error_t *
> >> +svn_subst_translate_cstring2(const char *src,
> >> +                             const char **dst,
> >> +                             const char *eol_str,
> >> +                             svn_boolean_t repair,
> >> +                             apr_hash_t *keywords,
> >> +                             svn_boolean_t expand,
> >> +                             apr_pool_t *pool)
> >> +{
> >> +  return translate_cstring2(dst, NULL, src, eol_str, repair, keywords, expand,
> >> +                            pool);
> >> +}
> >
> > So, this is revved because svn_subst_translate_string2() needs it (and
> > svn_subst_translate_string() doesn't).
>
> I am unsure of what you mean.
>
> I took the original implementation of `svn_subst_translate_cstring2`
> and placed it in the new static utility function `translate_cstring2`.
> `translate_cstring2` accepts another parameter, and its definition
> (the old definition on `svn_subst_translate_cstring2`) was
> appropriately modified to handle this new parameter. This way, I don't
> have to modify the public API.
>

Yes, yes. You revved three svn_subst_* functions; one publicly and two
privately. So I was pointing out (to myself), on each of the latter
two, where is the call site that needs the additional functionality.

> > I suggest you go ahead (respond to the review, submit an updated patch,
> > etc) without waiting; the svnsync part will eventually get its share of
> > reviews.  Also, you may want to consider splitting this patch into two
> > parts (subst work separate from svnsync work).
>
> I like the idea of splitting this into two patches.

Okay. Please post an updated patch to this list when you have it ready :-)

Best,

Daniel
Received on 2010-09-21 18:57:09 CEST

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