VCS Interface classes

[Matthew Woehlke]

Andreas Pakulat wrote:
> I present to you our new VCS interface classes (that I finally managed
> to write up, based on the enourmus thread some time ago).
> 
> * major problems for a given system with the proposed interface

IMO IBasicVCS is missing unedit(). Jakob already pointed out the gaping 
flaw in commit() ;-).

Come to think of it, it would rock if we handled change sets... yes, 
this is harder to do in e.g. svn since the plug-in would have to manage 
it rather than being able to leverage VCS functionality, but I know /I/ 
at least would find it really nice to have change sets in svn. :-)

Is VCSState::Unknown equivalent to 'not under version control', or 
should there be a seperate status (as I think there is in 3.4) for 
'unable to determine that information'? Or does that only need to exist 
in the UI side?

VCSMapping is insufficient for perforce which needs exclusion as well as 
inclusion rules. Is this just not available outside of a private (i.e. 
not-matching-the-interface), perforce-specific checkout()? (Or is it in 
the missing CliencMapping class? :-))

> * return types for the methods, we need a way to communicate at least
>   errors back, but I'm not sure how to do that best given that the 
>   methods should execute asynchronously

Change void to int, return 0 for success, <0 for error (possibly allow 
the return code to be implementation defined?). Or have an interface to 
retrieve an error.

I'm not sure about async, I would image async in a script is not really 
what you want :-). Therefore it seems like maybe we need both? Certainly 
I would guess there should be a statusAsync() based on previous 
discussion of svn in 3.4. IMO it would be ideal if we can require 
plug-ins to be thread safe and have a generic async wrapper around 
whichever plugin. Or just allow inheriting a base class that uses the 
virtual synchronous methods to do async and allow a plug-in to overload 
as needed?

For move():
> * depending on the VCS, only preserves history if copy preserves history

...I thought I remember someone saying this is not necessarily the case 
(as nonsensical as that seems), i.e. move might preserve history while 
copy does not? Anyway that's just a doc nitpick.

> * Parameters for push/pull in the distributed vcs iface, no idea whats
>   needed there
> * diff and log parameters, am not 100% sure there

Should diff() pop up a UI, or return something useful in scripts? With 
the latter, you could derive from a base class that implemented the UI 
version from the script-friendly version (like the above way of 
implementing async() methods. How had we originally planned to do this? 
I forget...). Also...

> * usage of simple QString's for any parameter that can be a repository
>   url (anything that is always a local url should stay KUrl), if a VCS
>   system can't use a url for describing a repository location

...if diff() takes a repo location shouldn't the parameter be QString? 
(Is it useful to have a KUrl /and/ QString version to make it easier to 
know if you have a url or a repo path?) Same comments for log().

-- 
Matthew
If you believe you received this e-mail in error, you are probably sadly 
mistaken, but if not, aren't you lucky?