VCS Interface classes

[Jakob Petsovits]

On Monday, 30. April 2007, Andreas Pakulat wrote:
> > * I think isValidDirectory() should be named isWorkingCopy(), because on
> > reading this name you instantly know what is asked for.
> > ("what does valid mean here?")
>
> Well, WorkingCopy seemed to be too svn-ish...

Well ok, whatever, it's just a cosmetic change anyways.
I thought the term to be used more widely, but I'm kinda SVN-biased as well.
Matthew, Kuba, your take?

> > * edit() is also not quite what the method is doing, how about
> > setEditable()?
>
> IMHO setEditable is not really proper, for me it always means setting a
> flag, which is not the case here. This may mean to fetch a file from the
> repository to make it editable.

Sounds reasonable, let's stay with edit().

> > * repositoryRevision() vs. localRevision() - what is the difference here?
>
> Simple: repositoryRevision() will give you the revision of that file on
> the repository, local will give you the local revision (i.e. svn info
> foobar, for example). This is the same after update() or commit() but
> might be different in other times.

Obviously doesn't apply to SVN and CVS (everything except the checksum stays 
the same), but if you say it's necessary for other VCSs, I'll believe you :)

> > Also, any particular reason why the parameter is restricted to local
> > copies?
>
> Well, repositoryRevision should really just check the revision of a
> local file in the repository, so I don't see the need to have a
> repository location here.

Ah right, makes sense.

> > * The above essentially also applies to diff(), but I'm not sure if it's
> > a good thing to make the parameters more complicated for such an edge
> > case.
>
> A list will be possible for most of the operations, i.e.
> add,remove,diff,update,log.

Oh cool, I totally forgot about those. Then again, everything but commit is 
less tragic because one composite action can easily be emulated by multiple 
atomic ones.

> > Question: what dstRevision will the method receive if the LocationDst is
> > a locally modified file inside the working copy? That's important to
> > document, as it's the primary use case.
>
> I don't think dstRevision should be used in that case, dstRevision is
> meant to be used when specifying a repository location. Maybe we
> shouldn't do this in 1 method but split those that can take either repo
> location or local one...

+1 for object oriented languages' method overloading feature :D
Like, an extra method for that standard use case:
virtual void diff( const KUrl& localLocation ) = 0;
(or the "list" version of that.)
I'm all for a split.

Had a short look at the other files, and they look fine - except for the 
distributed interface, I haven't got enough knowledge of how that works.

Cheers,
  Jakob