Make s3acl take an optional value #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So we can resort back to inheriting the bucket default setting instead of explicitly setting a value for it.
Unfortunately there is no direct conversion from scala Option to java Optional, so I can't get rid of the call.
laughedelic
reviewed
Sep 25, 2017
src/main/scala/SBTS3Resolver.scala
Outdated
| lazy val s3acl = settingKey[S3ACL]("Controls whether published artifacts are accessible publicly via http(s) or not") | ||
| lazy val s3optAcl = settingKey[Option[S3ACL]]("Controls published artifacts visibility via http(s) or inherits bucket setting") |
There was a problem hiding this comment.
I suggest a bit different approach: keep one key s3acl, change its type to Option[S3ACL] and add an implicit conversion S3ACL => Some[S3ACL]. Then you can deprecate that implicit conversion with an explanation that the type has changed and a suggestion to write Some(...) explicitly.
This way things also don't break (from users' point of view) and plus we keep the same setting key.
|
Seems to be complete now. I'm leaving the PR open for you to edit, just so you can bump the ivy-s3-resolver version. Thanks for helping and accepting the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So we can resort back to inheriting the bucket default setting instead of
explicitly setting a value for it.
This depends on ohnosequences/ivy-s3-resolver#20.
I'll fix the comments there and then update here. Still I wanted to open this so the
desing of this change can be debated as well.
Also, ref #54.