Malcolm Rowe <firstname.lastname@example.org> writes:
> So here's what I'm thinking of committing. Any comments?
Nice cleanups! Just a few comments:
> Miscellaneous cleanups for libsvn_wc/props.c.
> * subversion/libsvn_wc/props.c
> (conflicting_propchanges_p): Rename from
> and make static. Document that DESCRIPTION will be NULL if there
> is no conflict, and make it so.
> (get_existing_prop_reject_file): Rename from
> svn_wc__get_existing_prop_reject_file(), and make static.
> (svn_wc__merge_props): Followup to r17456. Rename local variable
> 'conflict_description' to 'conflict', and remove dependent boolean
> variable 'conflict', as the latter can be derived from the former.
> Update caller of get_existing_prop_reject_file().
> (svn_wc__merge_prop_diffs): Move local variable 'conflict_description'
> into the scope where it's used, and rename to 'conflict',
> again removing a dependent boolean variable. Update callers of
> conflicting_propchanges_p() and get_existing_prop_reject_file().
s/callers of/calls to/
> * subversion/libsvn_wc/props.h
> svn_wc__get_existing_prop_reject_file): Remove prototypes and doc-comments,
> as these functions are now static.
Very minor point: you could just say "Remove, as these are now static."
It goes without saying that when one removes a prototype, one removes
its doc string as well.
> --- subversion/libsvn_wc/props.c (revision 17460)
> +++ subversion/libsvn_wc/props.c (working copy)
> @@ -61,7 +61,7 @@
> /* Given two propchange objects, return TRUE iff they conflict. If
> there's a conflict, DESCRIPTION will contain an english description
> - of the problem. */
> + of the problem. Otherwise, *DESCRIPTION will be NULL. */
> /* For note, here's the table being implemented:
Heh, this doc string started out with problems: first, that it existed
at all, since (until now) it duplicated the doc string in the header
file. But that's no longer a concern, since the header doc is going
The second problem is that it imprecisely said "DESCRIPTION" instead
of "*DESCRIPTION". Your new text is more precise, but now the doc
string is inconsistent. Can you fix up the previous occurrence too?
(I've decided not to comment on passive voice: "DESCRIPTION will
contain" instead of just "set *DESCRIPTION to". See how good I'm
being, not commenting on that?)
> @@ -271,12 +273,11 @@
> -/* ### not used outside this file. make it static? */
> -svn_error_t *
> -svn_wc__get_existing_prop_reject_file (const char **reject_file,
> - svn_wc_adm_access_t *adm_access,
> - const char *name,
> - apr_pool_t *pool)
> +static svn_error_t *
> +get_existing_prop_reject_file (const char **reject_file,
> + svn_wc_adm_access_t *adm_access,
> + const char *name,
> + apr_pool_t *pool)
We've lost the doc string from props.h, so we shoul transfer it here.
(All functions should be documented, whether public, semi-public, or
Everything else looked good to me,
www.collab.net <> CollabNet | Distributed Development On Demand
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Mon Nov 21 18:23:33 2005