[Merge] lp:~tiagosh/telephony-service/indicator-generic-im-account into lp:telephony-service
Gustavo Pichorim Boiko
gustavo.boiko at canonical.com
Mon Aug 24 16:58:16 UTC 2015
Review: Needs Fixing
Just added one small note. Apart from that, do you think it would be possible to write unit tests for that?
Diff comments:
>
> === modified file 'indicator/textchannelobserver.cpp'
> --- indicator/textchannelobserver.cpp 2015-07-23 01:25:44 +0000
> +++ indicator/textchannelobserver.cpp 2015-08-19 18:05:54 +0000
> @@ -578,6 +587,11 @@
> if (!account) {
> return;
> }
> +
> + if (!account->account()->connection().isNull() &&
> + message.sender()->handle().at(0) == account->account()->connection()->selfHandle()) {
Can you just add a comment here explaining what exactly you are comparing? Right now we still remember what sender()->handle().at(0) means and why we had to do it this way, but we will certainly forget that soon .
> + return;
> + }
>
> // do not place notification items for scrollback messages
> if (mFlashChannels.contains(textChannel) && !message.isScrollback() && !message.isDeliveryReport() && !message.isRescued()) {
--
https://code.launchpad.net/~tiagosh/telephony-service/indicator-generic-im-account/+merge/262995
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.
More information about the Ubuntu-reviews
mailing list