Ktp-Image-Sharer library

Ahmed Ibrahim Khalil ahmedibrahimkhali at gmail.com
Mon Apr 7 09:16:30 UTC 2014


On Mon, Apr 7, 2014 at 2:16 AM, David Edmundson
<david at davidedmundson.co.uk>wrote:

> Ooh,
> This is a _lot_ more in depth and extensible than I was expecting. I
> guess this is a good thing, I'm just a bit surprised :)
>
> I don't think we want to put a library under the ktp-* umbrella that
> doesn't have much to do with KTp.
>
I think our options are either:
>  - make it generic
>  - put it in a folder in the text-ui.
>
>
Ok no problem with that I'll remove  "ktp-" prefix from the name, I thought
of trying to name it that way, as it is going to be used with other KTp
components (like the Chat plasmoid for example) instead of just copying the
library folder to each project as you suggested.

Right now I think it's small enough to be in a subfolder of the
> text-ui. Though I have had plans/daydreams about making a sharing lib
> that doesn't suck and can successfully replace KIPI and SLC and power
> the plasmoid and be used in all sorts of apps....so I'm not sure.
>
> Super fast review:
>  - is mform used?
>

Yeah, the MPForm class is part of the kipi-plugins project, it is used to
build multipart/form-data post body, it used across all the classes
inherits from AbstractSharer so I put it inside the AbstractSharer.

 - ShareProvider you leak your d pointer
>

No, the d pointer is deleted in the ShareProvider dtor. [1]
Please correct me if I was wrong.


>  - you don't have to do a deleteLater on KJobs they do it themselves
> (unless autoDelete is set to false)
>

I didn't know that, thanks for telling me. I am going to remove all the
deleteLater() calls of KJobs


>  - #include <X11/Xproto.h> ??
>

That seems a mistake that resulted from KDevelops auto complete feature and
that I didn't notice. Will remove it in the next commit.


>
> David
> _______________________________________________
> KDE-Telepathy mailing list
> KDE-Telepathy at kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
>


Finally, thanks a lot for spending time for the quick review. I am looking
forward for any other suggestions about the code that can make the library
better in terms of design or bug fixing.

Cheers
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140407/6235e24d/attachment.html>


More information about the KDE-Telepathy mailing list