On Thu, Sep 07, 2006 at 08:11:33PM +0530, Kamesh Jayachandran wrote:
> Move mergeinfo indexing code out of libsvn_fs_fs to libsvn_fs_util(new module)
> so that it can be reused by bdb repos.
>
It occurs to me that if you're creating a new utility library for
the filesystem implementations to use then we would want to consider
whether it's intended to be private (solely used by the filesystem
implementations) or public (usable by anyone). For example, should
users be able to call functions in it directly?
I'd suggest that we'd just be adding complexity if we made this a
publicly-accessible library: we'd have to support a whole new set of
interfaces in a binary-compatible fashion. On the other hand, if we
make it private, we can change the implementation at will.
It looks like you're planning to make it private, since you aren't
installing a public include file. I'd just like to make sure that that's
a deliberate choice rather than an accident of the current design.
I also wonder whether we really want to install a new shared library -
it might be better to compile libsvn_fs_util as a static library and
link it into both libsvn_fs implementations. That might require some
changes to the build generator, though.
If we do keep it as a shared library, you'll need to add version checks
(i.e. svn_ver_check_list()) to it, and also add it to the existing checks
in libsvn_fs_{base,fs}. If we make it static, that won't be necessary.
Finally, while I prefer the 'util' name, I wonder whether it'd be better
to call it something like 'libsvn_fs_subr' for parity with 'libsvn_subr'.
[The really important question - whether this change actually makes sense
in the context of the merge-tracking work - I'll leave to someone with
more experience of that code.]
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 7 18:01:51 2006