Review Request 109403: Add a new class for processing outgoing messages

[Daniele E. Domenichelli]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109403/#review28941
-----------------------------------------------------------


The code looks good, just a couple of comments...




CMakeLists.txt
<http://git.reviewboard.kde.org/r/109403/#comment21643>

    You will need to change KTP_MESSAGE_FILTER_FRAMEWORK_VERSION in all the plugins, or they won't work anymore.



KTp/abstract-message-filter.h
<http://git.reviewboard.kde.org/r/109403/#comment21642>

    Is there a reason to leave the context in this method?



KTp/pending-send-message.h
<http://git.reviewboard.kde.org/r/109403/#comment21644>

    PendingSendMessage doesn't look like a good name to me, it make me think about "Pending" stuff in TpQt that have a completely different meaning. Perhaps OutgoingMessage, or PlainTextMessage?


Also before a "ship it!" I'd like to see the other patch porting all the plugins to the new version :)

- Daniele E. Domenichelli


On March 11, 2013, 1:05 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109403/
> -----------------------------------------------------------
> 
> (Updated March 11, 2013, 1:05 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Add a new class for processing outgoing messages
> 
> Currently we re-use KTp::Message when the filters process a message to be sent, this exposes a lot of setters and getters that have no use at all.
> It also mixes HTML and plain text which has already led to one bug, and would probably lead to more.
> 
> This keeps the API cleaner and simpler
> 
> --
> 
> This is an API and ABI break in the middle of a feature freeze, which is normally against the results.
> I want this patch in because I want the API to be sensible for people to start writing their own plugins, I don't want them to break them for 0.7
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 323afb3c965f28eeea84f51956b7aa55a7b5588e 
>   KTp/CMakeLists.txt 448425593b6b1c2f0191c03e54871b7f7bc87ff5 
>   KTp/abstract-message-filter.h cf26907c5849b4df4ad725b2a5f7cb9f73953866 
>   KTp/abstract-message-filter.cpp 06d805db1e9e440ae95cdd0368c8ba3212c2bc4e 
>   KTp/message-processor.h a22fc707a22f9171728d859387d21eff99afdbe2 
>   KTp/message-processor.cpp 2e252f9e81b622c5673f0037cd127f3381a337b4 
>   KTp/pending-send-message.h PRE-CREATION 
>   KTp/pending-send-message.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109403/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130311/38845af0/attachment.html>