fvogt accepted this revision.
fvogt added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> broulik wrote in purposeplugin.cpp:86
> `errorCode` and `errorMessage` are forwarded from the Purpose plugin, `error`
> is our own. I could change `errorCode` to return a string from us then I
> guess. Or I hardcode a magic 1 here which is `ERR_USER_CANCELED` which
> Purpose uses when canceled but that won't help for the busy case?
Just call the argument `int errorCode` then, one variablename less
> purposeplugin.cpp:115
> + QJsonArray urls;
> + if (!urlString.isEmpty()) {
> + urls.append(urlString);
This whole if chain looks a bit weird - not having any items in the urls list
results in an error at the end, so why not bail out early and then refactor out
all `urls.isEmpty()` checks?
> purposeplugin.h:57
> + int m_pendingReplySerial = -1;
> +
> +};
Whitespace
REPOSITORY
R856 Plasma Browser Integration
REVISION DETAIL
https://phabricator.kde.org/D23151
To: broulik, #plasma, fvogt, davidedmundson, nicolasfella, apol, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2,
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg,
abetts, sebas, apol, mart
