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

Fix: Only wire upload mapping task if minifyEnabled #86

Merged
merged 4 commits into from
Apr 30, 2021
Merged

Fix: Only wire upload mapping task if minifyEnabled #86

merged 4 commits into from
Apr 30, 2021

Conversation

cerisier
Copy link
Contributor

@cerisier cerisier commented Apr 29, 2021

📜 Description

Fixes #82.

We only wire the upload mapping task if variant buildType has
minifyEnabled to true, which basically hints that proguard
rules will run.

Note that I got rid of variant.register since I believe we do not need it as we already wire the task manually. Happy to leave it there if needed.
Note that we still create the task eagerly for now, which is to be tackled in another PR.

💡 Motivation and Context

This follows a discussion with @marandaneto about #82

💚 How did you test it?

Published locally and ensured green build in debug variant where minifyEnabled is false.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

Fixes #82.

We only wire the upload mapping task if variant buildType has
minifyEnabled to true, which basically hints that proguard
rules will run.

Note that we still create the task for now.
@cortinico
Copy link
Contributor

Should we try to merge #85 instead of this one, wdyt @cerisier?

@cerisier
Copy link
Contributor Author

cerisier commented Apr 29, 2021

@cortinico Sorry I had quickl discussed with @marandaneto this afternoon and was suggesting I'd do a simple PR to fix the most urgent issue and take a bit more time to fix the eager task creation since this is going to be more complex given how things work atm.
Happy with both :)

@marandaneto
Copy link
Contributor

@cortinico Sorry I had quickl discussed with @marandaneto this afternoon and was suggesting I'd do a simple PR to fix the most urgent issue and take a bit more time to fix the eager task creation since this is going to be more complex given how things work atm.
Happy with both :)

let's go with this quick win and iterate over it, the other PR move things around but also includes this fix

@marandaneto marandaneto changed the title mappings: only wire upload mapping task if minifyEnabled Fix: Only wire upload mapping task if minifyEnabled Apr 30, 2021
@marandaneto marandaneto merged commit fa456d6 into getsentry:main Apr 30, 2021
@cerisier cerisier deleted the minify-enabled-dependency branch April 30, 2021 08:04
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

Successfully merging this pull request may close these issues.

Plugin fails for variants that generate no mappings
3 participants