Review Request: Activity ResourceInstance class for API review
Ivan Čukić
ivan.cukic at kde.org
Mon May 2 15:32:08 CEST 2011
> On May 2, 2011, 1:20 p.m., Aaron J. Seigo wrote:
> > experimental/libkactivities/ResourceInstance.h, line 77
> > <http://git.reviewboard.kde.org/r/101273/diff/1/?file=15799#file15799line77>
> >
> > sounds like a signal rather than a method.
> >
> > also, is this about the URI changing or the content that the URI represents changes?
> >
content changes
> On May 2, 2011, 1:20 p.m., Aaron J. Seigo wrote:
> > experimental/libkactivities/ResourceInstance.h, line 84
> > <http://git.reviewboard.kde.org/r/101273/diff/1/?file=15799#file15799line84>
> >
> > setUri? or even setUrl since it is a KUrl? also, should this take a QUrl to allow non-KUrl urls?
> >
> > also, this should be "const KUrl" in the signature, as there is no KUri class :)
URI as the name is used in nepomuk's stuff. I'm fine with making it QUrl
Yes :) KUri -> KUrl
> On May 2, 2011, 1:20 p.m., Aaron J. Seigo wrote:
> > experimental/libkactivities/ResourceInstance.h, line 105
> > <http://git.reviewboard.kde.org/r/101273/diff/1/?file=15799#file15799line105>
> >
> > agreed about it being a property with void setUrl(const KUrl&) and KUrl url() const for setting and getting.
Ok, although it doesn't imply that it is meant for the 'special cases'
> On May 2, 2011, 1:20 p.m., Aaron J. Seigo wrote:
> > experimental/libkactivities/ResourceInstance.h, line 64
> > <http://git.reviewboard.kde.org/r/101273/diff/1/?file=15799#file15799line64>
> >
> > imho, to follow our usual API style recommendations, the ctor should just be:
> >
> > ResourceInstance(QObject *parent = 0);
> >
> > setters and getters for QWidget (or better, perhaps: WId), URL and mimetype would then be provided.
With a convenience constructors? :)
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101273/#review3052
-----------------------------------------------------------
On May 2, 2011, 1:07 p.m., Ivan Čukić wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101273/
> -----------------------------------------------------------
>
> (Updated May 2, 2011, 1:07 p.m.)
>
>
> Review request for Nepomuk and Plasma.
>
>
> Summary
> -------
>
> Desc of the class is in the class apidocs
>
>
> Diffs
> -----
>
> experimental/libkactivities/ResourceInstance.h PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/101273/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110502/93af4574/attachment-0001.htm
More information about the Plasma-devel
mailing list