Code review comment for lp:~rls/bzr/transport-link-and-symlink-support

Revision history for this message
Martin Pool (mbp) wrote :

For clarity (especially for non unix programmers) I might s/link/hardlink

On 26 February 2010 02:50, Neil Santos <email address hidden> wrote:
> Review: Needs Information
> I've been wondering whether I should implement link() for LocalTransport using osutil's link_or_copy(); I figured it's what it's there for.  Is there any reason why I shouldn't do that?

That seems a bit like an abstraction inversion. If it's desirable
(sometimes?) to copy when you can't hardlink, then that should be done
across all transports. I don't think you would want this
unconditionally, so it should be either a different transport method
or an option to .hardlink(). Making it a separate hardlink_or_copy()
would allow it to be defined in the Transport base class without
needing to be overridden per transport.

--
Martin <http://launchpad.net/~mbp/>

« Back to merge proposal