sitter added inline comments.

INLINE COMMENTS

> KDECMakeSettings.cmake:74
>  # - ``APPLE_SUPPRESS_X11_WARNING`` option since 5.14.0
>  
>  
> #=============================================================================

Needs documentation for the l10n awesomeness.

> KDECMakeSettings.cmake:280
> +    if(KDE_L10N_DOWNLOAD_TRANSLATIONS)
> +        set(_EXTRA_ARGS "ALL")
> +    else()

This is not used anywhere, is it?

> KDECMakeSettings.cmake:302
> +
> +    add_custom_command(
> +        OUTPUT "${CMAKE_BINARY_DIR}/releaseme"

I wonder if we shouldn't use `ExternalProject` here. The advantage being that 
cmake would manage the clone and make sure it is updated as necessary. 
Disadvantageously, it doesn't seem to do `depth=1` presently 🐛, so I am not 
sure this would be a net-win.

  include(ExternalProject)
  ExternalProject_Add(releaseme
      PREFIX "${CMAKE_BINARY_DIR}/releaseme"
      GIT_REPOSITORY https://anongit.kde.org/releaseme.git
      CONFIGURE_COMMAND ""
      BUILD_COMMAND ""
      INSTALL_COMMAND ""
  )

(NOTE: dependable target would then be `releaseme`, also paths change with this)

REPOSITORY
  R240 Extra CMake Modules

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

To: apol, #frameworks, #build_system, kfunk, aacid, ltoscano
Cc: sitter

Reply via email to