Hi Daniel (Trebbien),

Just thought I would ask for an update, please.
I still have you on my "watch - list" - so there is no rush - just making sure 
for my own sanity that I haven't missed an email / thread about your patch 
submission.


Gavin "Beau" Baumanis


On 22/09/2010, at 2:56 AM, Daniel Shahaf wrote:

> 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...@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
> 

Reply via email to