Skip to content
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

(Idea) Lazily read APK path from variant #28

Closed
ZacSweers opened this issue Dec 10, 2014 · 8 comments
Closed

(Idea) Lazily read APK path from variant #28

ZacSweers opened this issue Dec 10, 2014 · 8 comments

Comments

@ZacSweers
Copy link
Contributor

Using our setup as an example, we do some processing in applicationVariants.all{...} to format the APK name with some extra information. This however breaks the plugin's ability to find the APK (due to it looking for the default name), so for now we've been using the solution you suggested in #17. This solution isn't really ideal though, as it makes the code depend on an internal field that could change in another release.

What would you think of allowing passing in a map of apkNames, where the key-value schema is defaultApkName -> customApkName. This way, we can inform the plugin what the new name is, and it can find it because it already has the old name.

I would be happy to take a crack at the idea in a pull request if you'd be amenable to it, let me know!

@ZacSweers
Copy link
Contributor Author

Here’s an example of how this would look in action, from another gradle plugin we use in our project.

@ZacSweers
Copy link
Contributor Author

Actually, after talking with that example library's developer a bit, he came up with an easier solution that might work here as well. Instead of getting the APK file upfront in apply(), he added an ApplicationVariant field to the upload task instead and sets the variant on that field instead. Then in the upload task (which runs almost last), it has the variant object and only then retrieves the APK file, which would have the updated name.

Here's the small commit where he implemented this. It's a fairly similar structure to this project as far as the configuration being in an apply method and the upload being a separate task.

What do you think?

@bhurling
Copy link
Contributor

Looks very interesting. I have already been looking for a way to honor the custom APK names. The lazy approach looks very promising. Shouldn't be too hard to implement.

@ZacSweers
Copy link
Contributor Author

Cool, please let me know how it goes or if you want me to take a shot at it :)

@bhurling bhurling changed the title (Idea) Allow specifying a defaultApkName-to-customApkName map in the plugin (Idea) Lazily read APK path from variant Dec 11, 2014
bhurling pushed a commit that referenced this issue Dec 11, 2014
Uses deprecated APIs for now so we should optimize it.
@ChristianKatzmann
Copy link
Member

@bhurling Is the branch lazy_apk_name ready for review? Should we open a PR for this?

@ZacSweers
Copy link
Contributor Author

I took a peak and left a comment with a couple suggested ways to avoid the
deprecation issue mentioned in the last commit on that branch, just
mentioning in case that's the only thing it's waiting on :)

On Mon, Dec 15, 2014, 2:24 AM Christian Becker notifications@github.com
wrote:

@bhurling https://github.com/bhurling Is the branch lazy_apk_name ready
for review? Should we open a PR for this?


Reply to this email directly or view it on GitHub
#28 (comment)
.

bhurling pushed a commit that referenced this issue Dec 15, 2014
@bhurling
Copy link
Contributor

I slightly changed the part where we find the apk file output:

def apkOutput = variant.outputs.find { variantOutput -> variantOutput instanceof ApkVariantOutput }

Just in case someone does not use the .apk extension in his custom filename. Also I dropped the part that checks for the zipAlign task. That part is already checked during the plugin setup.

What do you think?

@bhurling bhurling mentioned this issue Dec 15, 2014
@ZacSweers
Copy link
Contributor Author

Didn't even think of that. If it works then I say go for it!

On Mon, Dec 15, 2014, 2:58 AM Björn Hurling notifications@github.com
wrote:

I slightly changed the part where we find the apk file output:

def apkOutput = variant.outputs.find { variantOutput -> variantOutput instanceof ApkVariantOutput }

Just in case someone does not use the .apk extension in his custom
filename. Also I dropped the part that checks for the zipAlign task. That
part is already checked during the plugin setup.

What do you think?


Reply to this email directly or view it on GitHub
#28 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants