You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2022/08/17 09:18:46 UTC

[GitHub] [celix] PengZheng opened a new pull request, #439: Add conan option for bundle compression.

PengZheng opened a new pull request, #439:
URL: https://github.com/apache/celix/pull/439

   I deliberately choose compression as the default to avoid affecting current downstream conan package consumers, but I really don't mind making it consistent with the CMake default.


-- 
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-unsubscribe@celix.apache.org

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


[GitHub] [celix] pnoltes commented on pull request #439: Add conan option for bundle compression.

Posted by GitBox <gi...@apache.org>.
pnoltes commented on PR #439:
URL: https://github.com/apache/celix/pull/439#issuecomment-1233270415

   > I'll merge it so that I can deploy the latest Celix in my working environment.
   > 
   > If there is any change request, please leave it here. I'll address it ASAP.
   
   A bit late with my reaction, but I would prefer the same default behaviour from building with conan and cmake-only.
   And for me it is fine to make the default to use compression (indeed to prevent changes to the users) for both cmake and conan. 
   
   That being said, I actually think the PR #427 was merged too quickly. Because IMO configuring the jar/zip command arguments leaks too much details / requires too much knowledge from the users. 
   
   I think something like a CELIX_USE_COMPRESSION_FOR_BUNDLE_ZIPS/celix_use_compression_for_bundle_zips: [True, False] cmake/conan option is more user friendly. 
   
   @stegemr Do you agree with this and so yes, maybe we can update this I a new PR?
   
    


-- 
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-unsubscribe@celix.apache.org

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


[GitHub] [celix] PengZheng merged pull request #439: Add conan option for bundle compression.

Posted by GitBox <gi...@apache.org>.
PengZheng merged PR #439:
URL: https://github.com/apache/celix/pull/439


-- 
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-unsubscribe@celix.apache.org

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


[GitHub] [celix] PengZheng commented on pull request #439: Add conan option for bundle compression.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on PR #439:
URL: https://github.com/apache/celix/pull/439#issuecomment-1224108405

   I'll merge it so that I can deploy the latest Celix in my working environment.
   
   If there is any change request, please leave it here. I'll address it ASAP.


-- 
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-unsubscribe@celix.apache.org

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


[GitHub] [celix] PengZheng commented on a diff in pull request #439: Add conan option for bundle compression.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #439:
URL: https://github.com/apache/celix/pull/439#discussion_r956938822


##########
conanfile.py:
##########
@@ -78,6 +78,8 @@ class CelixConan(ConanFile):
         "celix_cxx": [True, False],
         "celix_install_deprecated_api": [True, False],
         "celix_add_deprecated_attributes": [True, False],
+        "celix_jar_command_arguments": ["ANY"],
+        "celix_zip_command_arguments": ["ANY"],

Review Comment:
   To support "ANY", newer conan is required. @pnoltes 



-- 
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-unsubscribe@celix.apache.org

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


[GitHub] [celix] PengZheng commented on pull request #439: Add conan option for bundle compression.

Posted by GitBox <gi...@apache.org>.
PengZheng commented on PR #439:
URL: https://github.com/apache/celix/pull/439#issuecomment-1233733965

   > IMO configuring the jar/zip command arguments leaks too much details / requires too much knowledge from the users.
   
   Totally agree, and I hate "ANY" options. I'll fix mess with a new PR.
   


-- 
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-unsubscribe@celix.apache.org

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