[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Moving svn_repos_deleted_rev to svn_fs_deleted_rev?

From: Blair Zajac <blair_at_orcaware.com>
Date: 2007-08-01 16:29:16 CEST

C. Michael Pilato wrote:
> Blair Zajac wrote:
>> Why is svn_repos_deleted_rev is in svn_repos.h and not in svn_fs.h?
>> This is new in 1.5, so we could move it still.
>>
>> It's the natural place I would look for that method, instead of
>> svn_repos.h.
>>
>> In fact, there's lots of interesting methods there that look like they
>> belong in svn_fs.h, not that we would move them now? They take
>> svn_fs_t, not svn_repos_t.
>>
>> What's the distinction between a method appearing in svn_fs.h and
>> svn_repos.h?
>
> Methods appear in svn_fs.h when their implementations live in libsvn_fs,
> libsvn_fs_fs_, and libsvn_fs_base. These libraries deal strictly with the
> versioned filesystem implementation. No hooks, no authz logic -- nothing
> that the repository layer adds on.
>
> Methods appear in svn_repos.h when their implementations live in libsvn_repos.
>
> So, the general rule here is, drop your function in the deepest layer that
> makes sense.

I guess there's a couple of ways to look at it:

If the implementation could be based on the public svn_fs.h API, then keep it in
svn_repos.h. This keeps svn_fs.h light

or

If the arguments only take svn_fs_t and not a svn_repos_t, then it belongs in
svn_fs_t.

Looking at some of the methods in svn_repos.h, the first thing they do is get
the svn_fs_t from the svn_repos_t and operate on that, so that would seem to
imply to move it into svn_fs.h.

> FWIW, I agree that svn_repos_deleted_rev should be svn_fs_deleted_rev().

I'll move it.

> I suspect that people creating these new functions are simply trying to keep
> "history reporting" functions lumped together in libsvn_repos without much
> thought as to proper layering.
>
> By the way, I see a really useful helper function just below
> svn_repos_deleted_rev(): check_ancestry_of_peg_path() could really stand to
> go public as svn_fs_check_ancestry() and get more generic by taking (path1,
> rev1, path2, rev2) instead of (path, pegrev, future_rev).

May not get to that one.

Regards,
Blair

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 1 16:28:00 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.