D17126: Privacy Options for Notifications Forwarded to Desktop

Simon Redman noreply at phabricator.kde.org
Sun Nov 25 19:37:47 GMT 2018


sredman added inline comments.

INLINE COMMENTS

> AppDatabase.java:166
> +            cv.put(KEY_PRIVACY_OPTIONS, value);
> +            // FIXME inserts data to a database, but getPrivacyOptionsValue() always returns 0
> +            if (res.getCount() > 0) {

I forgot to clean up this comment

> AppDatabase.java:177-191
> +    public void setPrivacy(String packageName, PrivacyOptions option, boolean isBlocked) {
> +        int curBit = option.ordinal();
> +        int value = getPrivacyOptionsValue(packageName);
> +        value |= (1 << curBit);
> +        value ^= isBlocked ? 0 : (1 << curBit);
> +        setPrivacyOptionsValue(packageName, value);
> +    }

These sections could do with some comments explaining how you are using the encoded bits to store the two privacy options.
Alternatively, it would be clearer to read if the table had two boolean columns instead of one integer :)

> NotificationsPlugin.java:262-264
> +            np.set("ticker", "");
> +            np.set("title", "");
> +            //np.set("text", "");

I was messing around with this (thus the commented-out line). Do any of these need to be here? I don't think so, but we should test. If not, then the "else" block which matches this "if" can probably be combined into the "if (!blockContents)" block starting at line 250

REPOSITORY
  R225 KDE Connect - Android application

REVISION DETAIL
  https://phabricator.kde.org/D17126

To: alexkovrigin, sredman, #kde_connect
Cc: sredman, alexkovrigin, kdeconnect, varunp, shivanshukantprasad, skymoore, brute4s99, wistak, dvalencia, rmenezes, julioc, Leptopoda, timothyc, jdvr, Danial0_0, johnq, Pitel, adeen-s, SemperPeritus, daniel.z.tg, jeanv, seebauer, bugzy, MayeulC, menasshock, mikesomov, tctara, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20181125/f824a8f4/attachment-0001.html>


More information about the KDEConnect mailing list