VCS: Proposal for a change of arguments to remove(), checkout() and import()
Fabian Wiesel
fabian.wiesel at googlemail.com
Fri Mar 27 07:26:33 UTC 2009
Hello,
starting with the simple suggestion: since add() has a
RecursionMode-parameter, I'd like to suggest adding the same
to remove() out of symmetry reasons. (Default removed for
brevity)
< VcsJob* remove( const KUrl::List& localLocations)
> VcsJob* remove( const KUrl::List& localLocations,
> RecursionMode recursion)
The next two changes are more far-reaching, hence my post
without a real patch:
Explicitly pass the source and target, instead of a VcsMapping to
checkout() and import()
<VcsJob* checkout(const VcsMapping &)
>VcsJob* checkout(const VcsLocation & sourceRepository,
> const KUrl & destinationDirectory, RecursionMode recursion)
<VcsJob* import(const VcsMapping &)
>VcsJob* import(const KUrl & sourceDirectory,
> const VcsLocation & destinationRepository, RecursionMode recursion)
Reasoning / Differences:
- VcsMapping provides more than one Repository / Local relation:
No VCS known to me provides a native support for such a
mapping, so every plugin has to implement loop over it.
Currently, no plug-in provides the capability and no
caller requires it. So, should such a behaviour ever be
required, I think, it should be rather be implemented outside
the interface.
- VcsMapping provides arbitrary sources and targets:
It allows too much flexibility to do the wrong things, as can
be seen by all the checks in the implementing functions and
it hides the real requirements to the caller.
I was wondering, does a non-recursive import make sense? CVS doesn't
seem to support it. I simply added it to maintain the same capabilities
of VcsMapping.
Hope, you find the changes acceptable.
Fabian
More information about the KDevelop-devel
mailing list