Julian Foad <julianf...@btopenworld.com> writes:
> Hi Philip. I hope you don't mind, I've made a few more review comments
> and suggestions, in the form of a patch.
It would be better if your mail program didn't convert tabs to hard
spaces, as it is I can't apply the patch.
> +/** ### Document here not what the callers will do but what this function
> + * itself is promised and what it is allowed/required to do.
I'm not really sure what you want.
> *
> - * @a path and @a lock are allocated in the result_pool passed to
> - * svn_fs_lock_many/svn_fs_unlock_many and so will persist beyond the
> - * callback invocation. @a fs_err will be cleared after the callback
> - * returns, use svn_error_dup() to preserve the error.
> - *
> - * If the callback returns an error no further callbacks will be made
> - * and svn_fs_lock_many/svn_fs_unlock_many will return an error.
> + * ### The callback invoked by svn_fs_lock_many() and svn_fs_unlock_many()
> + * ### [and svn_repos_fs_lock_many() and svn_repos_fs_lock_many() and
> others].
> + * ###
> + * ### @a path and @a lock are allocated in the result_pool passed to
> + * ### svn_fs_lock_many/svn_fs_unlock_many [and the others?] and so will
> persist beyond the
> + * ### callback invocation. @a fs_err will be cleared after the callback
> + * ### returns, use svn_error_dup() to preserve the error.
That documents the what the callback gets given, the callback can do
anything it likes with the data.
> + * ###
> + * ### If the callback returns an error no further callbacks will be made
> + * ### and svn_fs_lock_many/svn_fs_unlock_many will return an error.
That documents the only thing the callback can do that the caller cares
about.
> */
> typedef svn_error_t *(*svn_fs_lock_callback_t)(void *baton,
> const char *path,
> @@ -2553,9 +2564,6 @@ typedef svn_error_t *(*svn_fs_lock_callb
>
> /** Lock the paths in @a targets in @a fs.
> *
> - * @warning You may prefer to use svn_repos_fs_lock_many() instead,
> - * which see.
> - *
> * @a fs must have a username associated with it (see
> * #svn_fs_access_t), else return #SVN_ERR_FS_NO_USER. Set the
> * 'owner' field in each new lock to the fs username.
> @@ -2593,12 +2601,19 @@ typedef svn_error_t *(*svn_fs_lock_callb
> * For each path in @a targets @a lock_callback will be invoked
> * passing @a lock_baton and the lock and error that apply to path.
> * @a lock_callback can be NULL in which case it is not called.
> + * ### Implementation here requires lock_callback is non-null.
The implementation does
if (!cb_err && lock_callback)
cb_err = lock_callback(...)
which allows lock_callback to be NULL.
> The hunks in svn_repos.h are making the doc strings clearer by
> removing partial documentation of stuff that's already documented in
> the referenced FS-layer function. (The docs strings start with "Like
> svn_fs_...(), but ...".)
--
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
