D28192: WIP: Refactor dictionary runner

Harald Sitter noreply at phabricator.kde.org
Mon Apr 6 16:20:46 BST 2020


sitter added inline comments.

INLINE COMMENTS

> alex wrote in dictionaryrunner_config.cpp:29
> That makes sense but one question: The doc says: `...However, if you for some reason reimplement it and also are using KConfigXT, you must call this function`, does this mean we can assume that the base class is not needed?
> 
> PS: In this case it is not very relevant but I would like to understand concept for future patches 😃.

That line makes no assertions about what to do when you are not using KConfigXT. We don't really have consistent language for docs unfortunately but generally unless the docs explicitly say you must or you must not, it's usually best to assume that you should go with best practice.

In this particular case the fact that the docs say you must call the base when using kconfigxt could just mean that someone forgot about forwarding the call and then stumbled over bugs and took the time to add a warning for future generations to not repeat their mistake. It doesn't necessarily mean that the list of musts is comprehensive and exhaustive. It certainly doesn't mean that implicit requirements will remain the same over time. And along that line, consider the two scenarios:

in 10 years

- a new requirement is added where you must forward the call to the base. say a sound plays on every save and that is implemented inside KCModule. if you do not forward the call this singular KCM will be misbehaving and someone has to file a bug and a dev has to go figure out that the calls aren't being forwarded
- a new requirement is added where you must **not** forward the call (terribly unlikely) it's easy for the baseclass to know when that requirement was violated and Q_ASSERT or print a qwarning. at that point you'll still have a bug on your hand but the library can make provisions for that bug to not be actually a problem

or simply put: it's easier to deal with useless/unintended calls than with entirely missing calls. if you don't call a library it can't tell you about failed runtime assertions and the like.

REPOSITORY
  R114 Plasma Addons

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

To: alex, broulik, ngraham, sitter, mlaurent
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200406/b8d68a42/attachment-0001.html>


More information about the Plasma-devel mailing list