IBasicVersionControl (first try)

Hi!

I just started to port the CVS plugin to the new
IBasicVersionControl interface.

Here's what I had to do in order to make it compile:

*) Replaced KDEVVCS_EXPORT in IBasicVersionControl with
KDEVPLATFORMVCS_EXPORT as in IVersionControl

*) "Modified" is defined twice in KDevelop
namespace, so I renamed the second one to FileModified


As already discussed I also changed the return type of all
functions to VcsJob*


Please take a look at the attached patchfile and tell me if
it's ok for you.

Robert





-- 
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free
SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/
de/go/freemail

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

Robert Gruber wrote:
> *) "Modified" is defined twice in KDevelop
namespace, so I renamed the second one to FileModified

Shouldn't this be ItemModified? (And s/File/Item/g in the
doc of 
course...). Maybe we should change the others to e.g.
ItemAdded, etc. or 
at least e.g. Added for more consistency. (It might make
sense to only 
prefix *Modified; this makes it clear there is a difference
in object 
type for these, whereas the others apply to items or
directories.)

-- 
Matthew
Ideas in this e-mail are larger than they appear and the
writer may be 
smarter than he appears.


_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 30.05.07 15:04:58, Matthew Woehlke wrote:
> Robert Gruber wrote:
> > *) "Modified" is defined twice in
KDevelop namespace, so I renamed the second one to
FileModified
> 
> Shouldn't this be ItemModified? (And s/File/Item/g in
the doc of 
> course...). Maybe we should change the others to e.g.
ItemAdded, etc. or 
> at least e.g. Added for more consistency. (It might
make sense to only 
> prefix *Modified; this makes it clear there is a
difference in object 
> type for these, whereas the others apply to items or
directories.)

Its already called "Added", I agree we should
s/Foo/ItemFoo/ for the
VcsState.

Andreas

-- 
Your motives for doing whatever good deed you may have in
mind will be
misinterpreted by somebody.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 30.05.07 21:48:40, Robert Gruber wrote:
> I just started to port the CVS plugin to the new
IBasicVersionControl interface.

Thanks for trying that, I do know the VcsHelpers still need
some
implementation, I'll try to come up with something on the
weekend.

> Here's what I had to do in order to make it compile:
> 
> *) Replaced KDEVVCS_EXPORT in IBasicVersionControl with
KDEVPLATFORMVCS_EXPORT as in IVersionControl

Comitted right now as I was doing some cleanup anyway (hope
you don't
mind).

> As already discussed I also changed the return type of
all functions to VcsJob*

I already committed that one actually 

Andreas

-- 
An exotic journey in downtown Newark is in your future.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

Andreas Pakulat wrote:
> On 30.05.07 15:04:58, Matthew Woehlke wrote:
>> Robert Gruber wrote:
>>> *) "Modified" is defined twice in
KDevelop namespace, so I renamed the second one to
FileModified
>> Shouldn't this be ItemModified? (And s/File/Item/g
in the doc of 
>> course...). Maybe we should change the others to
e.g. ItemAdded, etc. or 
>> at least e.g. Added for more consistency. (It might
make sense to only 
>> prefix *Modified; this makes it clear there is a
difference in object 
>> type for these, whereas the others apply to items
or directories.)
> 
> Its already called "Added",

I'm looking at VcsAction.  (...which
should be Action, also.)

> I agree we should s/Foo/ItemFoo/ for the VcsState.

That isn't what I was talking about, but it makes sense for
VcsState. I 
guess then we leave VcsEvent::Action::Modified alone? (But
can we make 
them past-tense for consistency?)

Proposed patch attached. I moved Actions to avoid a circular
dependency 
(resolvable, but IMO it makes more sense for Actions to
belong to 
VcsItemEvent anyway).

For QFlags is it needed to specify values for the enum?

-- 
Matthew
Ideas in this e-mail are larger than they appear and the
writer may be 
smarter than he appears.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 30.05.07 17:00:15, Matthew Woehlke wrote:
> Andreas Pakulat wrote:
> >On 30.05.07 15:04:58, Matthew Woehlke wrote:
> >>Robert Gruber wrote:
> >>>*) "Modified" is defined twice in
KDevelop namespace, so I renamed the 
> >>>second one to FileModified
> >>Shouldn't this be ItemModified? (And
s/File/Item/g in the doc of course...). 
> >>Maybe we should change the others to e.g.
ItemAdded, etc. or at least e.g. 
> >>Added for more consistency. (It might make
sense to only prefix *Modified; 
> >>this makes it clear there is a difference in
object type for these, whereas 
> >>the others apply to items or directories.)
> >Its already called "Added",
> 
> I'm looking at VcsAction.  (...which
should be Action, also.)

Ok.

> >I agree we should s/Foo/ItemFoo/ for the VcsState.
> 
> That isn't what I was talking about, but it makes sense
for VcsState. I guess 
> then we leave VcsEvent::Action::Modified alone? (But
can we make them 
> past-tense for consistency?)

Yes.

> Proposed patch attached. I moved Actions to avoid a
circular dependency 
> (resolvable, but IMO it makes more sense for Actions to
belong to VcsItemEvent 
> anyway).

Agreed.

> For QFlags is it needed to specify values for the
enum?

Uhm, I think so, i.e. 0 1 2 4 8 16..

Please commit when you've added the values.

Andreas

-- 
You look like a million dollars.  All green and wrinkled.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

Andreas Pakulat agreed with my suggestions...:
> On 30.05.07 17:00:15, Matthew Woehlke wrote:
>> For QFlags is it needed to specify values for the
enum?
> 
> Uhm, I think so, i.e. 0 1 2 4 8 16..

I'm sure you meant to start with 1 .

> Please commit when you've added the values.

Done, and thanks.

-- 
Matthew
"943. I am not Bjorn of Borg."
  -- from 975 things Mr. Welch can no longer do in an RPG
http://theg
len.livejournal.com/16735.html


_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

Hi!

I just update and tried to compile with the changes you made
within the last 24 hours.

Again I've attached a patch with the things I had to do in
order to make it compile. Please take a look at it.

Although it compiles, I'm not completely sure if it is
correct. 
Due to the last changes VcsMapping::addMapping() takes a
parameter of 
type MappingFlags while VcsMapping::mappingFlags() returns 
just MappingFlag I wasn't sure which one is correct. So I
added
the missing Q_DECLARE_FLAGS(), OK?

I'm not fully sure what's the deal with the QFlags class and

the Q_DECLARE_FLAGS() macro, but why don't we use them on
all
enums?


Robert
-- 
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free
SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/
de/go/freemail

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

Robert Gruber wrote:
> I just update and tried to compile with the changes you
made within the last 24 hours.
> 
> Again I've attached a patch with the things I had to do
in order to make it compile. Please take a look at it.

Oops, some of those are my fault.  I checked
in all of those except 
the MappingFlag changes as they seem obvious.

The MappingFlag changes are obvious, too (I agree this
should be 
QFlags), but NonRecursive does not make sense as a flag. The
most 
obvious thing to do is simply drop it; any objections or
other suggestions?

-- 
Matthew
"943. I am not Bjorn of Borg."
  -- from 975 things Mr. Welch can no longer do in an RPG
http://theg
len.livejournal.com/16735.html


_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 31.05.07 14:23:37, Matthew Woehlke wrote:
> Robert Gruber wrote:
> > I just update and tried to compile with the
changes you made within the last 24 hours.
> > 
> > Again I've attached a patch with the things I had
to do in order to make it compile. Please take a look at
it.
> 
> Oops, some of those are my fault.  I checked
in all of those except 
> the MappingFlag changes as they seem obvious.
> 
> The MappingFlag changes are obvious, too (I agree this
should be 
> QFlags), but NonRecursive does not make sense as a
flag. The most 
> obvious thing to do is simply drop it; any objections
or other suggestions?

Yes, see my answer to Robert.

Andreas

-- 
You will be awarded some great honor.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel