You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2006/04/06 09:09:01 UTC

Re: svn commit: r19194 - in branches/fs-atomic-renames/subversion: include libsvn_delta libsvn_repos

rooneg@tigris.org writes:
 > Author: rooneg
 > Date: Wed Apr  5 17:37:49 2006
 > New Revision: 19194
 > 
 > Log:
 > On the fs-atomic-renames branch:
 > 
 > Rev the delta editor interface to support renames.  In order to keep
 > the diff small, this only revs the consumers of the delta editor that
 > are necessary to support dump/load, all other consumers use the old
 > version of the editor for the time being.

Oh, my! Here it goes:-)


 > * subversion/include/svn_delta.h
 >   (svn_delta_editor2_t): New editor, includes rename specific functions
 >    and documents that it must be created via a constructor.
 >   (svn_delta_default_editor2): Constructor for new editor.

Should we take the oportunity to call this svn_delta_editor_create to use
a similar name to many other constructor functions?


...and I didn't read the whole diff:-)

When you do the bulk of this: change everything to use the new editor,
will you move that work to trunk, then?  I really hope you will.
(Except for the rename stuff, of course).

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r19194 - in branches/fs-atomic-renames/subversion: include libsvn_delta libsvn_repos

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/6/06, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> On 4/6/06, Daniel Rall <dl...@collab.net> wrote:
> > On Thu, 06 Apr 2006, Peter N. Lundblad wrote:
> >
> > > rooneg@tigris.org writes:
> > ...
> > >  > Log:
> > >  > On the fs-atomic-renames branch:
> > >  >
> > >  > Rev the delta editor interface to support renames.
> > ...
> > > Should we take the oportunity to call this svn_delta_editor_create to use
> > > a similar name to many other constructor functions?
> >
> > +1, svn_delta_default_editor() is an aberration.
>
> That's fine with me.  Will make the change soon.

Well, it wasn't exactly soon, but I just committed the change in r19934.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r19194 - in branches/fs-atomic-renames/subversion: include libsvn_delta libsvn_repos

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/6/06, Daniel Rall <dl...@collab.net> wrote:
> On Thu, 06 Apr 2006, Peter N. Lundblad wrote:
>
> > rooneg@tigris.org writes:
> ...
> >  > Log:
> >  > On the fs-atomic-renames branch:
> >  >
> >  > Rev the delta editor interface to support renames.
> ...
> > Should we take the oportunity to call this svn_delta_editor_create to use
> > a similar name to many other constructor functions?
>
> +1, svn_delta_default_editor() is an aberration.

That's fine with me.  Will make the change soon.

> Also, should svn_delta_editor_to_editor2() really be in the public
> header file?  Seems like an implementation detail to me.

I was thinking it might be useful for users who had their own
svn_delta_editor_t's and wanted to pass them to new functions that
take svn_delta_editor2_t's, but I'm not overly sold on that either. 
Could certainly see making it private.

> ...
> > When you do the bulk of this: change everything to use the new editor,
> > will you move that work to trunk, then?  I really hope you will.
> > (Except for the rename stuff, of course).
>
> This might complicate subsequent porting of the backend renames
> changes, but if Garrett is up for that added pain, it would be nice to
> have these API changes sooner than later.

I've actually been leaning towards keeping them on the branch until
we're ready to at least merge the back end fs impls of svn_fs_move. 
It seems silly to jump through all the reving hoops and not get any
actual change out of it, which is what would happen if I merged now...

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r19194 - in branches/fs-atomic-renames/subversion: include libsvn_delta libsvn_repos

Posted by Daniel Rall <dl...@collab.net>.
On Thu, 06 Apr 2006, Peter N. Lundblad wrote:

> rooneg@tigris.org writes:
...
>  > Log:
>  > On the fs-atomic-renames branch:
>  > 
>  > Rev the delta editor interface to support renames.
...
> Should we take the oportunity to call this svn_delta_editor_create to use
> a similar name to many other constructor functions?

+1, svn_delta_default_editor() is an aberration.

Also, should svn_delta_editor_to_editor2() really be in the public
header file?  Seems like an implementation detail to me.

...
> When you do the bulk of this: change everything to use the new editor,
> will you move that work to trunk, then?  I really hope you will.
> (Except for the rename stuff, of course).

This might complicate subsequent porting of the backend renames
changes, but if Garrett is up for that added pain, it would be nice to
have these API changes sooner than later.
-- 

Daniel Rall