REMINDER: Final review of VCS interfaces

Matthew Woehlke mw_triad at users.sourceforge.net
Wed May 23 17:06:09 UTC 2007


Thanks for the review!

dukju ahn wrote:
> 3.
> virtual VcsJob IBasicVcs::status( const KUrl::List& localLocations,
>                      RecursionMode recursion ) = 0;
> 
> Is this an ASync method or blocking method? I guess it would be blocking
> because it fetches information from local-locations.

It's async, and needs to stay that way, perforce stores exactly zero 
local data, so status() needs to talk to the server and therefore should 
not be blocking.

> Then we need IBrowsableVcs::status( QString, RecursionMode ) also. And
> it should be non-blocking. Its associated signals should be defined too.

status() doesn't make sense in IBrowsableVC :-). Anyway all the signals 
are associated with VcsJob. See also Andreas' more detailed reply.

> 4.
> In case of svn, there is "svn info" It displays useful informations belows.
> Why don't we add showInfo() on both IBasic and IBrowsable and
> display information dialog box?

We would need to determine what "info" is common to all VCS's. IIRC the 
last time I tried to do that, the only thing that isn't already provided 
from some other interface was the repository root. I'm still a bit on 
the fence if we should have a repositoryRoot() function somewhere.

Otherwise feel free to add ISubversionVC::info() :-).

> logview should take revision range, both IBasicVC and IBrowsableVC

Umm... yes, IBrowsableVC::log() should take a revision range. Oops :-), 
thanks for the catch!

Before I fix it, any thoughts on folding the overloads into taking both 
limits at once? (Otherwise e.g. perforce /will/ totally ignore the 
revision limit and give you everything.)

-- 
Matthew
"I can hear you / just barely hear you / I can just barely hear you"
   -- "I Can Hear You", by They Might Be Giants





More information about the KDevelop-devel mailing list