On Mon, 23 May 2005, Julian Foad wrote:
> Peter N. Lundblad wrote:
> [...]
> >>* subversion/clients/cmdline/status.c
> >> (generate_status_desc): new function for getting detailed string
> >> representation of status
> >
> > You shouldn't describe the purpose of a new function in the log. That's
> > documented in the code.
>
> I think it's often good and useful to briefly mention the purpose of a new
> function in the log, as long as it is documented properly in the code as well.
>
Well, I don't feel strongly about this. I just think it is useless
duplication of information. and HACKING seems to prefer the "New
Function." idiom, even if it does't explicitly "forbid" mentioning the
purpose.
I think the important things to have in the log is what changed and the
purpose of the *change*, but not the purpose of every new function.
> >>* subversion/clients/cmdline/status-cmd.c
> >> (struct status_baton): added new item 'xml_mode' in struct
> >> for storing xml mode is requested or not
> >> (print_header_xml): prints xml header
> >> (print_footer_xml): prints xml footer
> >> (print_against_xml): prints status against version details in xml
> >
> > The three above functions are no. Please say so in the log.
>
> I think Peter means "new".
>
Of course:-) You know me? I'm the Master of Typos. :-(
> [...]
> >>+ /* "<entry>" */
> >>+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "entry",
> >>+ "path", path[0] == '\0' ? "." : path, NULL);
> >
> > svn_path_local_style takes care of this special casing for the empty
> > path. You should use that, since this comes from a local path
> > originally.
>
> I don't think we want backslashes in the XML when on Windows, for
> example, do we?
>
I think so. They're local paths after all. Just because we're generating
XML doesn't mean we should force the world to use POSIX-style path
delimiters. We did the same for the blame XML output. For other
commands, such as svn log, the paths are FS paths, which is different IMO.
Thanks for the review review:-)
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 24 08:27:58 2005