<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Apr 7, 2014 at 2:16 AM, David Edmundson <span dir="ltr"><<a href="mailto:david@davidedmundson.co.uk" target="_blank">david@davidedmundson.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Ooh,<br>
This is a _lot_ more in depth and extensible than I was expecting. I<br>
guess this is a good thing, I'm just a bit surprised :)<br>
<br>
I don't think we want to put a library under the ktp-* umbrella that<br>
doesn't have much to do with KTp. <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I think our options are either:<br>
- make it generic<br>
- put it in a folder in the text-ui.<br>
<br></blockquote><div> </div><div>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. <br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Right now I think it's small enough to be in a subfolder of the<br>
text-ui. Though I have had plans/daydreams about making a sharing lib<br>
that doesn't suck and can successfully replace KIPI and SLC and power<br>
the plasmoid and be used in all sorts of apps....so I'm not sure.<br>
<br>
Super fast review:<br>
- is mform used?<br></blockquote><div><br>Yeah, the MPForm class is part of the <span class="">kipi-plugins project, it is used to build </span><span class="">multipart/form-data post body, it used across all the classes inherits from AbstractSharer so I put it inside the AbstractSharer.<br>
<br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- ShareProvider you leak your d pointer<br></blockquote><div><br></div><div>No, the d pointer is deleted in the ShareProvider dtor. [1]<br></div><div>Please correct me if I was wrong.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- you don't have to do a deleteLater on KJobs they do it themselves<br>
(unless autoDelete is set to false)<br></blockquote><div><br></div><div>I didn't know that, thanks for telling me. I am going to remove all the deleteLater() calls of KJobs<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- #include <X11/Xproto.h> ??<br></blockquote><div><br></div><div>That seems a mistake that resulted from KDevelops auto complete feature and that I didn't notice. Will remove it in the next commit.<br></div><div>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><font color="#888888"><br>
David<br>
</font></span><div class=""><div class="h5">_______________________________________________<br>
KDE-Telepathy mailing list<br>
<a href="mailto:KDE-Telepathy@kde.org">KDE-Telepathy@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/kde-telepathy" target="_blank">https://mail.kde.org/mailman/listinfo/kde-telepathy</a><br>
</div></div></blockquote></div><br><br></div><div class="gmail_extra">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.<br>
<br></div><div class="gmail_extra">Cheers<br></div></div>