VcsJob question and suggestion

[Andreas Pakulat]

On 29.05.07 14:11:49, dukju ahn wrote:
> 1. name change
> 
> KJob has errorString() and kill(), which are same with
> VcsJob::errorMessage() and cancel().

Agreed.

> Although I know that VcsJob are not "officially" related with KJob,
> "practically" we, at least CVS and svn, may want to double-inherit from
> KJob and VcsJob both. ( I now agree that VcsJob is mere interface, not
> subclass of KJob )

Well, as I already said I don't really object having VcsJob inherit from
KJob. One plus of that is that we force vcs plugin developers to inherit
from KJob and thus all vcs plugins will have their actions shown in the
kjob-ui (i.e. kuiserver atm). Also this means VcsJob might become a real
class and not just an interface and thus make sure certain things work
as we want them to.

> So please change the VcsJob's two method name into KJob's name.

Just because Matthew and me are mainly working on the interfaces, that
doesn't mean nobody else should :)

> 2. QVariant VcsJob::fetchResults()
> 
> So, what is the actual type for QVariant?

Depends on the type() of the job.

> When status(), QVariant ( const QMap<QString, QVariant> & val ) ??
> and the QVariant at QMap should be VcsState value.

Yeap, I think that would work.

> When log(), annotate()  and diff(), what is the format of QVariant?

AFAICS:

log returns VcsEvent
annotate returns VcsAnnotation
diff returns ???

Seems like diff got lost along the way, basically it would be just a
small class around QPair<QString,QString> for text files. I recall that
Matthew and I talked about binary diffs returning a copy for both files
(original and new one), but that would need something different in the
QPair (not sure if QByteArray works).

> may this? QVariant qVariantFromValue ( const T & value )??

Yes that will be part of your implementation. Currently the VcsHelper
classes don't have an implementation and I'm not sure yet as to how all
of them will look like (because I'd like to create some of them only by
the plugins...), but the ones I mentioned above will be declared as
types using the QVariant macros.

> What if  other plain actions such as commit() add(), copy(), ??
> Is it worth for them to implement fetchResults() ?? I don't think so.
> Only retrieving FinishStatus would be sufficient for them.

Yes it is worth, to be on the safe side. However all you need to do is
return QVariant().

> 3.
> VcsJob should have finishStatus() also.

Well, if we have finishStatus() then we don't need the special
finished() signal and can go with the result() signal.

Andreas

-- 
You will be divorced within a year.