pnoltes commented on a change in pull request #401:
URL: https://github.com/apache/celix/pull/401#discussion_r831477835



##########
File path: bundles/cxx_remote_services/CMakeLists.txt
##########
@@ -15,21 +15,14 @@
 # specific language governing permissions and limitations
 # under the License.
 
-celix_subproject(REMOTE_SERVICE_ADMIN "Option to enable building the C++17 
Remote Service Admin Service bundles" OFF)
-if (REMOTE_SERVICE_ADMIN)
+celix_subproject(CXX_REMOTE_SERVICE_ADMIN "Option to enable building the C++17 
Remote Service Admin Service bundles" ON)

Review comment:
       Correct. IMO the CXX_REMOTE_SERVICE_ADMIN can be default OFF

##########
File path: test_package/CMakeLists.txt
##########
@@ -0,0 +1,270 @@
+#  Licensed to the Apache Software Foundation (ASF) under one

Review comment:
       Comment on the overall test_package dir.
   
   Maybe it is an idea to add this dir as a sub dir of examples and rename it 
to conan_usage_example and a small readme how this dir can be used standalone 
to create a project using Celix.
   
   I am also thinking about promoting this more in the introduction to Celix 
documentation, but this is for a different pull request. But this could be very 
nice for users so that they can quickly start using Celix. 




-- 
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.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

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


Reply via email to