Patch for preliminary Mercurial integration in kdevplatform

Andreas Pakulat apaku at gmx.de
Fri Mar 20 21:30:49 UTC 2009


On 16.03.09 22:22:54, Fabian Wiesel wrote:
> On Mon, 16 Mar 2009 11:04:49 +0100
> Andreas Pakulat <apaku at gmx.de> wrote:
> > Hmm, gimme a day or two to review the current vcscommon code and think
> > about this a bit [...]
> 
> Will do. At least on that matter :)

Sorry haven't had any spare minute to think on it, I'm still not sure
wether its better to have Svn,Git,Mercurial,CVS etc menus each with
delete, update, commit etc. but I'm actually open to simply try it out
:) So if anybody wants to come up with a patch I'll happily test and
apply it (won't have time to do it myself anytime soon)

Andreas
 
> > > > Its actually a url, so not necessarily a local file, it could just
> > > > as well be a file on a remote location - if the vcs plugin
> > > > supports that.
> > > 
> > > That is the problem: the naming (localURL(), etc) and the
> > > LocationType set by it suggest otherwise.
> > 
> > Granted this was written with mostly centralized vcs systems in mind
> > because nobody that wrote the code had lots of experience with dvcs
> > systems. So maybe we should actually remove VcsLocation from the
> > IBasicVC, splitting diff and the other methods that use it into two
> > interfaces: IBasicVC and ICentralizedVC. 
> >
> > So IBasicVC::diff() would take 2 KUrl's, IBasicVC::merge() would take
> > 1 KUrl (as local location) and two revisions as start/end point for
> > the merge-diff.
> >
> > What do you think?
> 
> This is going much further than I intended, but sounds good. I admit I
> am seeing some problems in fullfilling the diff() and merge()
> operations of IBasicVersionControl with mercurial as they stand, but

Can you elaborate on that a bit?

> VcsLocation is a perfectly okay, except in most DVCS (if not all) the
> fields reserved for describing a remote location are much less fitting
> a remote location than the field used for a local location.
> 
> Currently, I'd suggest removing the restriction, that an url is for a
> local location and renaming the members accordingly.

I think renaming localUrl() to simply "url()" would be fine. Can you
provide a patch? And while you're on it, can you check where the
constructor taking a QString is used? Doesn't seem to be overly useful
as there's a setter as well.

Andreas

-- 
Love is in the offing.  Be affectionate to one who adores you.




More information about the KDevelop-devel mailing list