guoyunhe marked 9 inline comments as done.
guoyunhe added inline comments.

INLINE COMMENTS

> davidre wrote in CMakeLists.txt:44
> Seems unused?

It is necessary for the KDED module. Without it, the compilation fails.

> davidre wrote in CMakeLists.txt:68
> unrelated

Will revert.

> davidre wrote in PoTD-list.txt:8
> Unrelated

It is related. It explains why sometimes APOD still fails to show in lock 
screen even after this fix.

> davidre wrote in cachedprovider.cpp:54
> Can't we save the image in its original format?

JPEG is the original format for all POTD providers. For the `save()` function, 
you must specify format or extension. It cannot guess the original format.

> davidre wrote in kded_potd.cpp:3
> QDebug

Will remove.

> davidre wrote in kded_potd.cpp:10
> unused

Will remove.

> davidre wrote in kded_potd.cpp:12
> You can use K_PLUGIN_CLASS_WITH_JSON

Changed

> davidre wrote in kded_potd.cpp:18
> this leaks

Fixed.

> davidre wrote in kded_potd.cpp:34
> Why don't we care if the data was updated?

We only want to trigger the caching behavior of POTD data engine. No need to do 
anything with the data itself.

> davidre wrote in kded_potd.cpp:46
> Why?

It won't work without this line. Each time you change and save lockscreen 
configuration, the `QFileSystemWatcher` stop watching the configuration file.

> davidre wrote in kded_potd.cpp:51
> Couldn't we get the config directly with "kscreenlockerrc" if we use 
> cascading either way? No need to seach for the actual path

I tried but it doesn't work.

> davidre wrote in kded_potd.h:21
> remove

Will do.

REPOSITORY
  R114 Plasma Addons

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

To: guoyunhe, #plasma, davidre, broulik
Cc: davidre, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to