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

Reply via email to