You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2001/03/10 00:50:11 UTC

Re: CVS update: subversion/subversion/libsvn_ra_local ra_plugin.c

On Thu, Mar 08, 2001 at 06:00:17PM -0000, sussman@tigris.org wrote:
>   User: sussman 
>   Date: 01/03/08 10:00:17
> 
>   Modified:    subversion/include svn_ra.h
>                subversion/libsvn_ra_local ra_plugin.c
>   Log:
>   * svn_ra.h (get_commit_editor): take base_path argument, for the same
>     reason that svn_fs_get_editor now does.  Namely, the client already
>     knows the "grandaddy" dir where it will call replace root.  It's
>     silly to force the editor-driver to always begin changes at the very
>     top of a revision tree.

I don't understand this part. Why isn't the root just placed at BASE_PATH?

In other words, don't open the session with "/A" and then pass "B/C/D" as
BASE_PATH. Just open the session with "/A/B/C/D".

Tossing more and more parameters at the function is a sign of weakness, IMO.
A solid abstraction should not require a bazillion parameters to control its
operation. (more params == more variability == more potential for problem)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: CVS update: subversion/subversion/libsvn_ra_local ra_plugin.c

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Mar 10, 2001 at 05:51:24AM -0600, Ben Collins-Sussman wrote:
> Greg Stein <gs...@lyra.org> writes:
> > On Thu, Mar 08, 2001 at 06:00:17PM -0000, sussman@tigris.org wrote:
>...
> > >   * svn_ra.h (get_commit_editor): take base_path argument, for the same
> > >     reason that svn_fs_get_editor now does.  Namely, the client already
> > >     knows the "grandaddy" dir where it will call replace root.  It's
> > >     silly to force the editor-driver to always begin changes at the very
> > >     top of a revision tree.
> > 
> > I don't understand this part. Why isn't the root just placed at BASE_PATH?
> > 
> > In other words, don't open the session with "/A" and then pass "B/C/D" as
> > BASE_PATH. Just open the session with "/A/B/C/D".
> 
> Ohhhhh, I think see what you're saying!  
> 
> The fs commit editor *requires* that you specify a REVISION:BASE_PATH
> pair whet you fetch it.  This is the place where you want to start
> editing the filesystem (what replace_root() will affect.)
> 
> But: this doesn't mean the client needs to see this requirement.  As a
> user of RA, it already *has* a way to say where it wants to start
> editing: the ra->open() call takes a URL.  Let the client provide
> BASE_PATH within this URL, and provide REVISION to
> ra->get_commit_editor() as usual.  Then the RA layer can put 1 and 1
> together when fetching the fs commit editor
> 
> Am I understanding correctly?  :)

Yessir! Spot on :-)

You have all the information that you need passed into RA (via two call
points: open, get_commit), so adding more parameters to the interface is
just adding complexity.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: CVS update: subversion/subversion/libsvn_ra_local ra_plugin.c

Posted by Ben Collins-Sussman <su...@newton.ch.collab.net>.
Greg Stein <gs...@lyra.org> writes:

> On Thu, Mar 08, 2001 at 06:00:17PM -0000, sussman@tigris.org wrote:
> >   User: sussman 
> >   Date: 01/03/08 10:00:17
> > 
> >   Modified:    subversion/include svn_ra.h
> >                subversion/libsvn_ra_local ra_plugin.c
> >   Log:
> >   * svn_ra.h (get_commit_editor): take base_path argument, for the same
> >     reason that svn_fs_get_editor now does.  Namely, the client already
> >     knows the "grandaddy" dir where it will call replace root.  It's
> >     silly to force the editor-driver to always begin changes at the very
> >     top of a revision tree.
> 
> I don't understand this part. Why isn't the root just placed at BASE_PATH?
> 
> In other words, don't open the session with "/A" and then pass "B/C/D" as
> BASE_PATH. Just open the session with "/A/B/C/D".

Ohhhhh, I think see what you're saying!  

The fs commit editor *requires* that you specify a REVISION:BASE_PATH
pair whet you fetch it.  This is the place where you want to start
editing the filesystem (what replace_root() will affect.)

But: this doesn't mean the client needs to see this requirement.  As a
user of RA, it already *has* a way to say where it wants to start
editing: the ra->open() call takes a URL.  Let the client provide
BASE_PATH within this URL, and provide REVISION to
ra->get_commit_editor() as usual.  Then the RA layer can put 1 and 1
together when fetching the fs commit editor

Am I understanding correctly?  :)