[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