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

Fixed crashes with Tasker plugin actions when using minified code #543

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

joaomgcd
Copy link
Contributor

@joaomgcd joaomgcd commented Nov 4, 2022

Description

The Tasker plugin code has an issue that only occurs when code is minified (ie in release builds) that is fixed here. This bug is preventing the Tasker actions from being used at all.

Testing Instructions

  1. Build app with minifyEnabled true
  2. Try using any Pocket Casts Tasker action from Tasker
  3. Check that it doesn't crash anymore

@joaomgcd joaomgcd requested a review from a team as a code owner November 4, 2022 09:00
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I didn't even think about the class files getting stripped out when I was reviewing this initially. Good reminder to be careful about that. Thanks for tracking down the issue and fixing this!

I'm not sure we'll be able to get this into 7.26, so could you add a 7.27 release note, and then we can merge this?

@joaomgcd
Copy link
Contributor Author

joaomgcd commented Nov 4, 2022

Ok, updated. Thanks.

A shame the first release with Tasker integration doesn't work. 😣Is the version with Tasker already available for everyone? Or just in beta?

Thanks again!

@mchowning
Copy link
Contributor

Is the version with Tasker already available for everyone? Or just in beta?

It's in beta.

If we have to update the beta for any other reason, we'll try to get this in too, but since this isn't a regression (on account of it never working in release builds 😞 ) I don't think this alone quite justifies cutting a new beta release (there's a fair bit of work around cutting a new release, and it would mean that all of our beta users would be downloading another apk, which we try to minimize).

@mchowning mchowning enabled auto-merge November 4, 2022 13:14
@joaomgcd
Copy link
Contributor Author

joaomgcd commented Nov 4, 2022

Sure, I understand. Since it's in beta I don't think it's that big of an issue. I was getting worried that everyone in PR would have a broken Tasker integration. 😅 Hope it works now!

@mchowning
Copy link
Contributor

I was getting worried that everyone in PR would have a broken Tasker integration.

They will when the beta gets released as 7.26 in a week or so. 😞 Fortunately, they never had Tasker integration before, so it's at least not breaking existing functionality. We'll keep this in mind though, and see if we can get it in before 7.26 is released.

@mchowning mchowning merged commit 99ff95c into Automattic:main Nov 4, 2022
@joaomgcd
Copy link
Contributor Author

joaomgcd commented Nov 4, 2022

Oh I see. That is unfortunate... Maybe it's best that this is tested on a real release build then, just to make sure that we didn't miss anything else related to it being built that way.

Unfortunately I'm not able to do a proper release build, I always get the following error:

Execution failed for task ':app:processReleaseGoogleServices'.
> No matching client found for package name 'au.com.shiftyjelly.pocketcasts'

Any way around that? Would be great to make sure that everything is working in a proper installation from a release APK.

@mchowning
Copy link
Contributor

mchowning commented Nov 4, 2022

Any way around that? Would be great to make sure that everything is working in a proper installation from a release APK.

I'm not sure there's a way around it short of you creating your own google project with our release package name and getting your own google-services. json file from that (to be clear, I'm not suggesting you actually do that 😄, but I'm not sure there's a better way for you to make an actual release build ). But I did test this with a release build when I reviewed the PR this time, and it worked fine, so I think we're good.

@joaomgcd
Copy link
Contributor Author

joaomgcd commented Nov 4, 2022

Awesome! :) Thank you for testing!

ashiagr pushed a commit that referenced this pull request Nov 5, 2022
Fixed crashes with Tasker plugin actions when using minified code
ashiagr pushed a commit that referenced this pull request Nov 5, 2022
Fixed crashes with Tasker plugin actions when using minified code
@ashiagr ashiagr added this to the 7.26 ❄️ milestone Nov 5, 2022
@ashiagr
Copy link
Contributor

ashiagr commented Nov 5, 2022

This is included in a new beta 7.26-rc-4.

@joaomgcd
Copy link
Contributor Author

joaomgcd commented Nov 7, 2022

Thank you very much for including it early!!

@joaomgcd joaomgcd deleted the tasker_plugin_fix_for_minify branch November 15, 2022 08:03
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.

3 participants