Garrett Rooney <rooneg@gmail.com> writes:
> Still on the todo list:
> More comments
> Command line client help output
> Reduce the code duplication in the various log functions
> Implement ra_svn and ra_dav versions
> Maybe make get_log_bounded functions take an array of paths, not just one.
> Maybe add backwards compatability code so this can work with older servers?
And log message, I hope :-).
Quick comments -- this was not as thorough a review as I'd like, but
something's better than nothing I guess:
> Index: subversion/include/svn_repos.h
> ===================================================================
> --- subversion/include/svn_repos.h (revision 10134)
> +++ subversion/include/svn_repos.h (working copy)
> @@ -538,6 +538,25 @@
> apr_pool_t *pool);
>
> /**
> + * @since New in 1.1
> + *
> + * Call @a history_func (with @a history_baton) for each interesting
> + * history location in the lifetime of @a path in @a fs, for @a num_back
> + * revisions starting at @a start and going backwards. Only cross
> + * filesystem copy history if @a cross_copies is @c TRUE. And do all
> + * of this in @a pool.
> + */
> +svn_error_t *
> +svn_repos_history_bounded (svn_fs_t *fs,
> + const char *path,
> + svn_repos_history_func_t history_func,
> + void *history_baton,
> + svn_revnum_t start,
> + unsigned int num_back,
> + svn_boolean_t cross_copies,
> + apr_pool_t *pool);
> +
> +/**
Why not change 'num_back' to a signed number 'distance', and reverse
its default sense? That way the API can always go some distance
behind *or* ahead of a given revision.
In terms of documenting this, just say it's "same as calling
svn_repos_history() with end == start + distance" or something like
that (with the right formatting codes, of course).
> * @since New in 1.1.
> *
> * Set @a *locations to be a mapping of the revisions to the paths of
> @@ -626,6 +645,18 @@
> apr_pool_t *pool);
>
>
> +svn_error_t *
> +svn_repos_get_logs_bounded (svn_repos_t *repos,
> + const char *path,
> + svn_revnum_t start,
> + unsigned int num_back,
> + svn_boolean_t discover_changed_paths,
> + svn_boolean_t strict_node_history,
> + svn_log_message_receiver_t receiver,
> + void *receiver_baton,
> + apr_pool_t *pool);
> +
Same comments here.
> /* ---------------------------------------------------------------*/
>
> /**
> Index: subversion/include/svn_ra.h
> ===================================================================
> --- subversion/include/svn_ra.h (revision 10134)
> +++ subversion/include/svn_ra.h (working copy)
> @@ -733,6 +733,15 @@
> apr_array_header_t *location_revisions,
> apr_pool_t *pool);
>
> + svn_error_t *(*get_log_bounded) (void *session_baton,
> + const char *path,
> + svn_revnum_t start,
> + unsigned int num_back,
> + svn_boolean_t discover_changed_paths,
> + svn_boolean_t strict_node_history,
> + svn_log_message_receiver_t receiver,
> + void *receiver_baton,
> + apr_pool_t *pool);
> } svn_ra_plugin_t;
This needs to be documented like other RA vtable functions, of course.
> Index: subversion/include/svn_opt.h
> ===================================================================
> --- subversion/include/svn_opt.h (revision 10134)
> +++ subversion/include/svn_opt.h (working copy)
> @@ -197,7 +197,10 @@
> svn_opt_revision_working,
>
> /** repository youngest */
> - svn_opt_revision_head
> + svn_opt_revision_head,
> +
> + /** an offset relative to another revision */
> + svn_opt_revision_offset
> };
I guess this whole bit might change now, with the suggestion of
"-rHEAD:HEAD-10" syntax that others have made. (So I won't comment on
any of the parsing code.)
As far as backwards compatibilty: When the server is new enough to
understand the new type of request, there should be no extra round
trip. When the server is old, it can error back to the client, which
the client can detect, and only *then* do an extra round-trip to
get the offset revision by number so it can re-request.
So, here's a wicked question: shouldn't an offset of "HEAD-10" on a
file show you the last ten revisions *in which that file changed*, not
simply the revisions numbered HEAD through HEAD-10? :-) (Or did your
code already do that, and I just missed it?)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jul 5 21:05:31 2004