[Differential] [Updated] D817: Some includes and CmakeLists fixed

[kossebau (Friedrich W. H. Kossebau)]

kossebau added a comment.

Good morning :)
Looks good in general, just some comments where things could be optimized for minimal cmake config code.

Okteta just got a fix for the missing installed header, both master and Application/15.12 branches.
Did not yet try to compile KDevelop with this diff though, so cannot give feedback on it.
Please tell me if the fix commited to Okteta works for you.


INLINE COMMENTS
  utils/CMakeLists.txt:1 TODO note can be removed with this work :)
  utils/CMakeLists.txt:2 Almost all of the below can be removed.
  Actually the only lib which needs to be searched for explicitely is `OktetaKastenControllers`.
  Its config cmake dependency chain requires and imports all the others already (unless something else regressed, poke me if).
  So this should be enough for this whole file (untested):
  ```
  find_package( OktetaKastenControllers 0.3.1)
  set_package_properties(OktetaKastenControllers PROPERTIES
          PURPOSE "Required for building Okteta KDevelop plugin."
          URL "https://kde.org/"
          TYPE OPTIONAL)
  
  if (OktetaKastenControllers_FOUND)
      add_subdirectory(okteta)
  endif()
  ```
  
  If you want to be a little bit more explicit, you could require
  ```
  find_package( OktetaGui )
  find_package( KastenControllers )
  find_package( OktetaKastenControllers )
  ```
  to reflect the three logical groups here (OktetaCore/Gui, KastenCore/Gui/Controllers, OktetaKastenCore/Gui/Controllers). But then it does not really add value, so you could leave that out.
  utils/CMakeLists.txt:32 Why remove these? The subdir should be only entered to build the plugin if the optional deps are found, no?
  It rather needs fixing to use the proper `_FOUND` vars (matching the lower/uppercasing of the package name(s)), and just `if (OktetaKastenControllers_FOUND)` should be enough here
  utils/okteta/CMakeLists.txt:30-32 To be removed now.
  utils/okteta/CMakeLists.txt:38-42 Just `OktetaKastenControllers` should be enough. If you prefer explicit listing of all libs, the complete list is
  ```
  OktetaKastenControllers
  OktetaKastenGui
  OktetaKastenCore
  KastenControllers
  KastenGui
  KastenCore
  OktetaGui
  OktetaCore
  ```
  utils/okteta/CMakeLists.txt:49-50 Can be removed now as well, give there is no more desktop file generated and used.
  utils/okteta/kastentoolviewwidget.cpp:28 If you prefer, CamelCase includes should now be possible for all classes, so e.g. here
  ```
  #include <Kasten/Okteta/ByteArrayView>
  ```
  and similar for all other includes.
  
  But plain header includes like you kept using for now work of course as well. Just in case you preferred them, for consistency :)

REPOSITORY
  rKDEVELOP KDevelop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apuzio, kfunk, kossebau
Cc: kdevelop-devel