Here is a quick review of the public APIs marked "@since New in 1.2" in r13616,
and of the doc strings of the APIs deprecated by them.
Enhancement: Ought to use "depth" instead of "recurse".
Enhancement: Ought to use "depth" instead of "descend" or "recursive".
Maybe "descend" and "recursive" ought to be renamed to "recurse" for
Enhancement: Ought to use "depth" instead of "nonrecursive".
Maybe "nonrecursive" should be renamed to "recurse", and its sense inverted,
Enhancement: Ought to update all targets from the same repository to the
Bug: doesn't mention svn_client_checkout2's "ignore_externals" argument.
Bug: doesn't mention svn_client_update2's "ignore_externals" argument.
Bug: doesn't mention svn_client_propset2's "ctx" argument.
Bug: doesn't mention svn_client_export3's "ignore_externals" argument.
Bug: doesn't mention svn_client_export3's "recurse" argument.
Bug: Needs to say how it relates to svn_repos_get_logs3.
Bug: parameter "force" needs a better name (and, in some cases,
description?). In some cases, such as svn_fs_unlock, it feels like the name
"force" is good enough and can have no other interpretation, but I think that's
an illusion and it should be renamed anyway.
I'm not terribly happy with this. I don't think it makes clear what "server
mode" is and why.
Someone raised a concern about whether these belong in svn_error.h.
Need to clarify the difference between these, or merge them.
Why has the error number changed? Isn't that an ABI violation if we are now
returning a different number for semantically the same error? Shouldn't the
new name be added, equal to the old name, and the default message be changed to
the new one?
It is not clear that this necessarily refers to a locking-related error (as
svn_error_is_lock_error thinks it does).
No need to say "username must be non-NULL".
Contradiction: "@a lock->owner must be filled in with an authenticated
username. If not, ...". Change "must" to "may".
Contradiction: "If @a lock->path is already locked, then return @c
SVN_ERR_FS_PATH_LOCKED. If @a force is true, then steal the existing lock
anyway, ...". Change to "... unless @a force is true, in which case ..." or
Contradiction. Before "return @c SVN_ERR_FS_LOCK_OWNER_MISMATCH", add "and
@a force is false,".
Clarification: change "do_lock should be" to "do_lock is".
Use of type "long int" is not generally helpful in a public interface.
Plain "int" would be better.
Bug: Says "Like @c svn_fs_get_locks" but it isn't much like it; it returns
the result in a completely different, undocumented way.
Bug: Needs to mention "LF" in the list of newline types.
Each time a group of new things is preceded by "@since New in 1.2.", it
ought probably to have Doxygen group markers around it, else it is not clear
what items the "@since" refers to.
There's a nastily formatted note. Just make it plain text or "@warning".
The description of "flags" is a tiny bit vague; it should state explicitly
that the argument is to be composed from the mentioned constants.
Bug: The "@since" above it needs to be part of its Doxygen comment.
SVN_ERR_* (in svn_error_codes.h) (maybe)
Bug: The comment should be a Doxygen comment (starting "/**").
Bug: Mis-use of "@c" where "@a" is meant in doc string.
Bug: Bogus doc strings. (Note that "@a" should only be used for arguments
of _this_ function.)
The documentation for the structure fields should be in the structure
definition rather than in the function's doc string. Or, at least, the
structure doc string should refer to the function for details.
Bug: The "err" field is not documented.
Typo: unclosed tag "</tt"; also "an" -> "a" on the same line.
Typo: argument "set_Locks_baton" -> "set_locks_baton".
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com
Received on Thu Mar 24 02:25:07 2005