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