Sir Woody Hackswell <sir.woody@hackswell.com> writes:
> I'd like to request an addition to the libsvn_ra API.
>
> I'd like to add a function similar to get_log, but instead of returning
> logs, return revision numbers and paths. These routines follow
> the get_log() interface very closely, but return just the revision numbers
> and paths. (revision node). These routines will always traverse path
> changes.
You are trying to basically marshal the output of the new FS history
code (specifically, the results of svn_fs_history_location) back to
the client, which is a worthy cause. But I have some problems with
several points of this proposal.
> These routines could be used to help diff and log out when
> traversing back among renames. These routines are also required to
> implement the -r PREVn interface I'm working on. But one thing at a
> time, no? :)
I'm not excited in the least about the PREVn concept. But sure, one
thing at a time.
> <svn_ra.h> (and ra_* plugins)
> /** Get at most "maxrevisions" from each path in "paths", starting at
> * "start" and ending at "end", following path changes.
> *
> * Results are returned in the APR array "revisions". Elements are of
> * type svn_revnode_t.
> */
> svn_error_t *(*get_revisions) (void *session_baton,
> const apr_array_header_t *paths,
> svn_revnum_t start,
> svn_revnum_t end,
> int maxrevisions,
> apr_array_header_t *revisions,
> apr_pool_t *pool);
You don't need the END -- your bounded by MAXREVISIONS. And having
only a single START is asking for the same trouble we're running into
with the get_log() interface. You probably a START for each PATH.
Finally, do you really need all the intermediate revisions? If
someone does PREV1000, all you really care about is the result that's
1000 revisions back in time, right? So way make the network marshal
back all the extra info, to be accumulated into a big ol' array by the
RA layer, then only to get discarded by the client?
Further, you have a bunch of PATHS going into this function, and a
bunch of REVISIONS coming out. But there is a 1-to-many mapping of
these things as you've defined the interface. How are you going to
keep track of which REVISIONS go with which PATH, especially since the
paths can change? That's right -- you're not. A better interface
would return a hash, keyed on the original PATH inputs, with arrays of
svn_revnode_t's as values, and probably promise something about the
sorting of those arrays. Or, if you don't need the intermediate
revisions, a hash keyed on original PATH inputs with a single
svn_revnode_t value per key.
> <libsvn_repos/revisions.c>
> svn_error_t *
> svn_repos_get_revisions (svn_repos_t *repos,
> const apr_array_header_t *paths,
> svn_revnum_t start,
> svn_revnum_t end,
> int maxrevisions,
> apr_array_header_t *revisions,
> apr_pool_t *pool);
See above complaints.
You're definitely going to need to more thoroughly outline your PREVn
design ideas and your supposed enhancements to diff and log before
getting anyone to sign off on a brand new API this late in the
ballgame.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 25 22:11:17 2003