davidedmundson marked an inline comment as done. davidedmundson added inline comments.
INLINE COMMENTS > dfaure wrote in kconfigtest.cpp:1800 > typo: mimics > > The fact that it's not actually another process, means that this test would > pass with a simple emit as well ;) > A proper test would use QProcess to run a helper executable which would make > changes to a config file. Fixed the typo Any test would still pass if you had explicit code that cheated to make tests pass! I want to add "--notify" support in kwriteconfig5. That currently lacks any tests, I can do that which would cover proving that this is sending it properly, whilst keeping this more advanced test still readable. > dfaure wrote in kconfig.cpp:465 > I wonder if we should skip doing this work when DBUS support is disabled, for > performance reasons. > More #if, but more speed... in the corner-case scenario though. > So I'm not sure ;) We still have the loop regardless. It doesn't make sense for apps to use notify if they're running in a standalone environment. So practically I think we'd only save an if statement on a flag. > dfaure wrote in kconfigbase.h:62 > Let's go for 51 :) Sure, I'll change this when I merge. Typing @since like this is the only strategy that hasn't caused headcaches with the fastpaced frameworks cycle. It's easier to grep for than a number. > dfaure wrote in kconfigwatcher.cpp:20 > (I'm curious, why not a raw pointer? You clean up the hash when the watcher > is destroyed anyway; probably doesn't matter either way though) I'm returning QSharedPointers to the caller. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D13034 To: davidedmundson, broulik, dfaure Cc: dfaure, broulik, zzag, kde-frameworks-devel, michaelh, ngraham, bruns
