D26856: Removing deprecated warnings messages

[Friedrich W. H. Kossebau]

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


  Hi @lbiaggi, welcome to contributing to KDevelop, thanks for your first(?) patch proposed here. I agree with your goal to make the code more modern :)
  
  Sadly I have to start with some nitpicks about this patch, but you are here to also learn what others think about development, right? :)
  
  First thing: the commit message is mainly something read by people iin the future. When will they read it, and how much help will they have from the text we give to them? The current text describes the current initial motivation "get rid of warnings". That though is not our actual motivation. We rather want to make the code use the recommended & latest API, when possible. That should be noted here.
  
  But then there is also a mix of changes happening in this very patch which are done for different reasons.
  E,g the formatting changes or the reordering of includes. How are they relating to the actual commit message? I guess there is no relation. So they will have to be removed from this very patch.
  
  Then, ideally each porting to new API is done in separate commits, one per API change ported to. This makes commits more easy to read when looking back from the future, and the commit message can be precise and short about the very change. Do a line-by-line git blame call and see yourself how useful precise & compact commits and messages are, and how confusing commits with rather unrelated mass changes are and respective unhelpfll commit messages.
  So please do separate commits for each case.
  
  Also did you not mention how you created this patch, and if you tested your changes, and how?

REPOSITORY
  R32 KDevelop

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

To: lbiaggi, kossebau
Cc: kossebau, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20200122/ea92a24c/attachment.html>