kossebau requested changes to this revision.
kossebau added a comment.
This revision now requires changes to proceed.


  Having slept over this one night, I still think that this change should be 
not the way to go to fix the seen issues.
  
  The actual problem is bad usage in application code, usually due to lack of 
knowledge what is needed.
  The documented API of KAboutData, especially  AboutData::setDesktopFileName() 
<https://api.kde.org/frameworks/kcoreaddons/html/classKAboutData.html#a112d2fc20c31e7847995930e030cc67b>,
 is very explicite what needs to be done and when. This patch would change this 
behaviour and runs a chance to break things for application code which relies 
on the documented behaviour.
  Changing this behaviour to help application code whose authors never bothered 
to get things only means screwing over those application developers who did the 
effort to read up in the API dox and write proper code.
  
  If we were to redesign this API, the behaviour proposed in this patch would 
be fine IMHO. So IMHO please add a comment to change things for KF6, as that 
the desktopfilename should be auto-generated not in the constructor and then be 
fixed, but from the currently set component name and org domain. Unless set 
explicitely to a value.
  
  What could be done here, is to add some example code to the class API dox, 
for the section "Detailed Description", which would serve as some template code 
how to use the existing API for application metadata, together with the needed 
desktopfile name and/or D-Bus names. And then write a blog post to try to catch 
more people's awareness of this issue, pointing out also all applications which 
have been fixed already.
  
  Some additional unit tests for KAboutData around the issue would be good to 
have as well.

REPOSITORY
  R244 KCoreAddons

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

To: stikonas, mpyne, ltoscano, kossebau, aacid
Cc: plasma-devel, kde-frameworks-devel, #frameworks, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol

Reply via email to