You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by GitBox <gi...@apache.org> on 2021/01/06 22:43:43 UTC

[GitHub] [jclouds] JnRouvignac opened a new pull request #93: Simplify S3 code using java-xml-builder

JnRouvignac opened a new pull request #93:
URL: https://github.com/apache/jclouds/pull/93


   - animal sniffer should be on java18, just like `<jdk.version>`
   - Only use elem() and text() to have similar looking code
   - Remove unnecessary call to up() because the returned value is never used
   - Simplify code
   - Deduplicate code
   - Make the code more explicit by returning the rootBuilder


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



[GitHub] [jclouds] nacx commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
nacx commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-758507979


   @gaul any chance you could run the aws-s3 tests for this PR? I think the change is safe, but I'm wondering why this very old code manually copied the ACL objects.


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



[GitHub] [jclouds] gaul commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
gaul commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-764625057


   Successfully ran the integration tests against aws-s3.  We are interested in any OSGi improvements, if they remove dependencies or otherwise.  Thank you for your contribution @JnRouvignac!


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



[GitHub] [jclouds] JnRouvignac commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
JnRouvignac commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-757814524


   No, unfortunately I do not have an AWS account, so I can't directly test myself :(


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



[GitHub] [jclouds] JnRouvignac edited a comment on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
JnRouvignac edited a comment on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-762705311


   BTW are you interested in removing the dependency on `XMLBuilder`?
   
   This adds a dependency to the `com.jamesmurty.utils:java-xmlbuilder` jar and a transitive dependency to the `net.iharder:base64`  jar (which is superseded by `java.util.Base64` with java 8 anyway). They are 18kb each approximately and not OSGi compatible. They are not huge, but it's less API surface and it's less things to change when trying to use jclouds in an OSGi context (they need to be replaced by OSGi compatible bundles like `org.apache.servicemix.bundles.java-xmlbuilder`).
   
   Of course that would be a separate PR once this PR gets approved and merged.


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



[GitHub] [jclouds] gaul merged pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
gaul merged pull request #93:
URL: https://github.com/apache/jclouds/pull/93


   


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



[GitHub] [jclouds] JnRouvignac commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
JnRouvignac commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-764754029


   Excellent! Thank you very much!
   I'll write to the mailing list when I'll have a little more time to work on removing these dependencies.


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



[GitHub] [jclouds] JnRouvignac commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
JnRouvignac commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-764754029


   Excellent! Thank you very much!
   I'll write to the mailing list when I'll have a little more time to work on removing these dependencies.


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



[GitHub] [jclouds] gaul commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
gaul commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-764625057


   Successfully ran the integration tests against aws-s3.  We are interested in any OSGi improvements, if they remove dependencies or otherwise.  Thank you for your contribution @JnRouvignac!


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



[GitHub] [jclouds] gaul merged pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
gaul merged pull request #93:
URL: https://github.com/apache/jclouds/pull/93


   


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



[GitHub] [jclouds] JnRouvignac commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
JnRouvignac commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-762705311


   BTW are you interested in removing the dependency to `XMLBuilder`?
   
   This adds a dependency to the `com.jamesmurty.utils:java-xmlbuilder` jar and a transitive dependency to the `net.iharder:base64`  jar (which is superseded by `java.util.Base64` with java 8 anyway). They are 18kb each approximately and not OSGi compatible. They are not huge, but it's less API surface and it's less things to change when trying to use jclouds in an OSGi context (they need to be replaced by OSGi compatible bundles like `org.apache.servicemix.bundles.java-xmlbuilder`).
   
   Of course that would be a separate PR once this PR gets approved and merged.


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



[GitHub] [jclouds] nacx commented on pull request #93: Simplify S3 code that uses java-xml-builder

Posted by GitBox <gi...@apache.org>.
nacx commented on pull request #93:
URL: https://github.com/apache/jclouds/pull/93#issuecomment-757762909


   LGTM. Have you been able to run the live tests against aws-s3?


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