-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add distribution support to gradle-plugin #297
base: main
Are you sure you want to change the base?
Conversation
b4a7497
to
a9ea833
Compare
5faa01b
to
c82c886
Compare
c82c886
to
566216e
Compare
apiKey.set(System.getenv("DISTRIBUTION_API_KEY")) | ||
|
||
// Optional, defaults to 'release' | ||
tag.set("release") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag doesn't seem to be relevant currently - I'd suggest removing it until we have an upload-related task around distro to avoid confusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag is used to define the release 'channel' (when the SDK checks for an update we send the tag and only consider builds a with matching tag). Possibly we should have a distribution upload task but I was worried that just makes it more confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put in my thoughts on seeing if we can simplify the API for the initial release. I’m not too familiar with the product yet so feel free to push back.
|
||
/** | ||
* The key used to authenticate downloads for future updates. | ||
* Emerge recommends setting this as an environment variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, my thought on simplifying this API would be that we should remove this API and instead just check the environment variable DISTRIBUTION_API_KEY
.
If a customer needs a way to set this via the extension, then we should add it.
@@ -294,6 +295,17 @@ class EmergePlugin : Plugin<Project> { | |||
) | |||
addItem("tag (optional): ${extension.reaperOptions.tag.orEmpty()}", reaperHeading) | |||
|
|||
val distributionHeading = addHeading("distribution") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might need rebasing, i moved this logExtension function to a new task in a recent change.
/** | ||
* The list of build variants Distribution is enabled for. | ||
*/ | ||
abstract val enabledVariants: ListProperty<String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m just thinking about this from a perspective of simplifying this API, what would be the downside to enabling distribution for all variants? Can we remove this API from the initial release?
No description provided.