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
Received on 2008-07-09 21:26:59 CEST