celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [celix] abroekhuis commented on a change in pull request #254: Refactors generation and usage of CelixTargets cmake files
Date Tue, 09 Jun 2020 16:39:43 GMT

abroekhuis commented on a change in pull request #254:
URL: https://github.com/apache/celix/pull/254#discussion_r437169729



##########
File path: cmake/cmake_celix/BundlePackaging.cmake
##########
@@ -845,9 +851,12 @@ function(install_celix_bundle_targets)
         set(EXPORT_FILE ${EXPORT_NAME}BundleTargets)
     endif ()
 
-#    if (NOT CMAKE_BUILD_TYPE STREQUAL "Release")
+    set(BASE_EXPORT_FILE ${EXPORT_FILE})
+    if (CMAKE_BUILD_TYPE)
         set(EXPORT_FILE ${EXPORT_FILE}-${CMAKE_BUILD_TYPE})
-#    endif ()
+    else ()
+        set(EXPORT_FILE ${EXPORT_FILE}-noconfig)

Review comment:
       Isn't it cleaner to use the trick that is done further down? If no build type, set
build type to "NOCONFIG" and use that everywhere? Limits the build_type check to only one
place.

##########
File path: cmake/cmake_celix/BundlePackaging.cmake
##########
@@ -201,13 +201,19 @@ function(add_celix_bundle)
     endif ()
 
 
-    set(BUNDLE_FILENAME ${BASE_BUNDLE_FILENAME}-${CMAKE_BUILD_TYPE}.zip)
-    foreach (NO_POSTFIX_BT IN LISTS CELIX_NO_POSTFIX_BUILD_TYPES)
-        if (CMAKE_BUILD_TYPE STREQUAL NO_POSTFIX_BT)
-            #setting bundle file name without postfix
-            set(BUNDLE_FILENAME ${BASE_BUNDLE_FILENAME}.zip)
-        endif ()
-    endforeach ()
+    if (CMAKE_BUILD_TYPE)

Review comment:
       Can cmake_build_type be not set at all?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message