Of course, the appropriate patch now (after r32043, in which Julian
reverted his change) would be somewhat different anyway...
-Karl
Daniel Shahaf <d.s_at_daniel.shahaf.co.il> writes:
> Senthil Kumaran S wrote on Wed, 9 Jul 2008 at 03:11 +0530:
>> Karl Fogel wrote:
>> > Julian, maybe r32015/r32018/r32021 should be redone to put this code in
>> > libsvn_subr and svn_xml_private.h instead? I know you didn't want to
>> > put property-specific code in the 'svn_xml__' namespace, but that would
>> > appear to be by far the lesser evil here... Thoughts?
>>
>> How about the attached patch.
>>
>
> I didn't read the patch, just the log message, but I have a few comments
> nonetheless:
>
>> [[[
>> Follow up to r32015, r32018, r32021, r32022.
>>
>> Move XML-printing function from libsvn_client to libsvn_subr.
>>
>
> Why move it? (I know why; but the log message should say that.)
>
> Also, I think a link to this thread would be useful.
>
>> * build.conf
>> (svnlook): Remove libsvn_client dependency.
>> (libsvn_client): Remove svn_client_private.h from msvc-export.
>>
>> * subversion/libsvn_subr/cmdline.c
>> (): Include svn_string.h, svn_xml.h, svn_config.h
>> (svn_cmdline__print_xml_prop): New function.
>>
>
> It isn't "new". It is svn_client__print_xml_prop, moved and renamed;
> that's what the log message should say. It should be clear from the log
> message which symbols are new and which were renamed, and what their old
> and new names are.
>
> e.g.,
>
> (svn_cmdline__print_xml_prop): New name for svn_client__print_xml_prop().
>
> You'll also see this form used:
>
> * foo.c (foo_func): Move to..
> * bar.c (bar_func): ..here.
>
>> * subversion/libsvn_client/xml.c
>> Removed.
>>
>
> Please list here the public symbols that existed in this file and what
> was done with them (were they deleted, moved, or renamed, and where to).
>
>> * subversion/svn/props.c
>> (): Remove svn_client_private.h
>> (svn_cl__print_xml_prop_hash): Use svn_cmdline__print_xml_prop instead of
>> svn_client__print_xml_prop.
>>
>> * subversion/svn/propget-cmd.c
>> (): Remove svn_client_private.h
>> (print_properties_xml): Use svn_cmdline__print_xml_prop instead of
>> svn_client__print_xml_prop.
>> (svn_cl__propget): Same as above.
>>
>> * subversion/include/svn_cmdline.h
>> (svn_cmdline__print_xml_prop): New API.
>>
>
> It should be declared in include/private/svn_cmdline_private.h.
>
>> * subversion/include/private/svn_client_private.h
>> Removed.
>>
>> * subversion/svnlook/main.c
>> (): Removed svn_client_private.h
>> (do_plist): Use svn_cmdline__print_xml_prop instead of
>> svn_client__print_xml_prop.
>>
>> Patch by: stylesen
>> Suggested by: dglasser
> ^
> s/d//
>
>> ]]]
>>
>> Thank You.
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-09 21:52:15 CEST