Re: [Patch] Issue #2291 - 'svn ls -v' return locking information - V2

Here's a review of your log message.

Alexander Thomas wrote:
> Version 2 of the patch for fixing Issue #2291 ('svn ls -v' return
> locking information).
>
> [[[
> Fix issue #2291 - 'svn ls -v' return locking information.

"return" -> "should return"

>
> * subversion/include/svn_types.h
> (struct svn_dirent2_t): Added new struct.

"(svn_dirent2_t): New struct."

By including the word "Added", you implied that the change you made to
svn_dirent2_t was adding a new struct (perhaps inside it).

> (struct svn_dirent_t): Deprecated and re-arranged.

Omit "struct"; just give its name.

How and why was it rearranged? Actually, I think maybe you mean it was moved
(whereas "rearranged" implies that parts of it were moved).

>
> * subversion/libsvn_ra_local/ra_plugin.c
> (svn_ra_local__get_dir): Modified and re-arranged.

"Modified" isn't helpful. Obviously you modified it; that's why it is
mentioned in this log message. Instead, say briefly why and how you modified
it, and why you rearranged it. For example, "Simplified by using the new
helper function 'foo', and rearranged so that the most important results are
output first." (That is a completely made-up example.)

>
> * subversion/clients/cmdline/ls-cmd.c
> (print_dirents): Modified to print lock information.

You can just say "Print lock information."

> (print_dirents_xml): Modified to print lock information in xml.

Just say "Output lock information."

> (svn_cl__ls): Modified to call new function.

What new function? You haven't mentioned a new function. And why - for what
purpose or effect?

>
> * subversion/clients/cmdline/main.c
> Modified help text for svn ls.

"Mention locking information in the help text for 'svn ls'." would be better.

>
> * subversion/clients/cmdline/dtd/list.dtd
> Modified dtd file.

"Updated DTD file." would be better.

>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_get_dir): Modified and re-arranged.

Again, a better explanation is needed.
>
> * subversion/libsvn_ra_dav/fetch.c
> (svn_ra_dav__get_dir): Modified.

And here.

> ]]]
>
>

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 5 20:38:55 2005