D16894: [ECM] use a macro to add compiler flags conditionally

[Christophe Giboudeaux]

cgiboudeaux added inline comments.

INLINE COMMENTS

> ECMAddCompilerFlag.cmake:1-25
> +#=============================================================================
> +# Copyright 2018      René J.V. Bertin <rjvbertin at gmail.com>
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#

Shall be moved after the module documentation

> ECMAddCompilerFlag.cmake:27-29
> +include(CMakeParseArguments)
> +include(CheckCXXCompilerFlag)
> +include(CheckCCompilerFlag)

Shall be moved after the license block

> ECMAddCompilerFlag.cmake:30-58
> +
> +# ECM_ADD_CXX_COMPILER_FLAGS_IF_SUPPORTED(FLAGS <flag|flags>
> +#     [IF_SUPPORTED <support_condition>]
> +#     [QUERY_IF <query_condition>]
> +# )
> +# add <flag> or <flags> to CMAKE_CXX_FLAGS if the compiler supports them.
> +# Support is determined by the IF_SUPPORTED expression if provided or by

Sphinx cannot parse this doc.

Also don't forget to create ECMAddCompilerFlag.rst in docs/modules/

> ECMAddCompilerFlag.cmake:58
> +#     [QUERY_IF <query_condition>]
> +# )
> +

Missing "Since 5.xx"

> ECMAddCompilerFlag.cmake:61
> +
> +function (ECM_ADD_CXX_COMPILER_FLAGS_IF_SUPPORTED)
> +    set(_OPTIONS_ARGS)

Unneeded extra space

> ECMAddCompilerFlag.cmake:66
> +
> +    cmake_parse_arguments(EASCXXFLAGS "${_OPTIONS_ARGS}" "${_ONE_VALUE_ARGS}" "${_MULTI_VALUE_ARGS}" ${ARGN} )
> +    if(NOT EASCXXFLAGS_FLAGS)

Unneeded extra space

> ECMAddCompilerFlag.cmake:68
> +    if(NOT EASCXXFLAGS_FLAGS)
> +        message( FATAL_ERROR "ecm_add_cxx_compiler_flags_if_supported: 'FLAGS' is a required argument." )
> +    endif()

Unneeded extra spaces

> ECMAddCompilerFlag.cmake:96
> +
> +function (ECM_ADD_C_COMPILER_FLAGS_IF_SUPPORTED)
> +    set(_OPTIONS_ARGS)

Unneeded extra space

> ECMAddCompilerFlag.cmake:101
> +
> +    cmake_parse_arguments(EASCFLAGS "${_OPTIONS_ARGS}" "${_ONE_VALUE_ARGS}" "${_MULTI_VALUE_ARGS}" ${ARGN} )
> +    if(NOT EASCFLAGS_FLAGS)

Unneeded extra space

> ECMAddCompilerFlag.cmake:103
> +    if(NOT EASCFLAGS_FLAGS)
> +        message( FATAL_ERROR "ecm_add_c_compiler_flags_if_supported: 'FLAGS' is a required argument." )
> +    endif()

Unneeded extra space

REPOSITORY
  R240 Extra CMake Modules

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

To: rjvbb, #build_system, kfunk
Cc: cgiboudeaux, dfaure, kfunk, apol, kde-frameworks-devel, kde-buildsystem, #build_system, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20190130/aa2b8b1e/attachment-0001.html>