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

Remove Jetifier #56

Merged
merged 5 commits into from
Mar 15, 2022
Merged

Remove Jetifier #56

merged 5 commits into from
Mar 15, 2022

Conversation

0nko
Copy link
Contributor

@0nko 0nko commented Mar 10, 2022

This PR replaces the Tenor library dependency reference with a jetified AAR version.

To test:
Verify the library and the sample app can be built and that the GIF search works when running the sample app (you'll need an API key)

@ParaskP7 ParaskP7 self-assigned this Mar 10, 2022
@ParaskP7 ParaskP7 added the enhancement New feature or request label Mar 10, 2022
Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @0nko !

This is great, I just reviewed and tested the PR, everything looks good to me. I also double-checked with Bye bye Jetifier and it was successful, thank you! 🚀 🎉 🍾

To test:
Verify the library and the sample app can be built and that the GIF search works when running the sample app (you'll need an API key)

Btw, I tested the sample app, with an without an API key, and to my suprise both builds was working when I tried to search for a GIF even when having <ADD API KEY HERE> as the TENOR_API_KEY key... 🤷 😅 🤔

@ParaskP7
Copy link
Contributor

👋 @0nko !

It is still a 👍 🟢 from my side, maybe @malinajirka wants to take a look as well, so I am letting you two merge this PR.

PS: About the Lint issue that you fixed by updating the outdated dependencies, in my other PR here, I actually didn't do the update, but instead suppressed the GradleDependency rule for the sample app (see 0605a8c commit). I did that because IMHO I think Lint shouldn't fail due to an outdated dependency, I believe it is too much of an expectation. 🤔

@0nko
Copy link
Contributor Author

0nko commented Mar 14, 2022

Thanks, @ParaskP7!

Btw, I tested the sample app, with an without an API key, and to my suprise both builds was working when I tried to search for a GIF even when having as the TENOR_API_KEY

Huh, that's weird 😆

About the Lint issue that you fixed by updating the outdated dependencies, in my #55, I actually didn't do the update, but instead suppressed the GradleDependency rule for the sample app (see 0605a8c commit). I did that because IMHO I think Lint shouldn't fail due to an outdated dependency, I believe it is too much of an expectation

Yeah.. But here it's not as much of a problem since it's much simpler to check if any of the flows might get broken by an update. So I'd say let's keep the newer versions. But I'll think about suppressing the errors next time.

@ParaskP7
Copy link
Contributor

👋 @0nko !

Huh, that's weird 😆

😅

Yeah.. But here it's not as much of a problem since it's much simpler to check if any of the flows might get broken by an update. So I'd say let's keep the newer versions. But I'll think about suppressing the errors next time.

True! 👍

PS: Let me merge this now in order to progress with this Jetifier work and my in-progress Gradle & AGP upgrades, thank you! 🚀

@ParaskP7 ParaskP7 merged commit d260931 into trunk Mar 15, 2022
@ParaskP7 ParaskP7 deleted the remove-jetifier branch March 15, 2022 08:25
@ParaskP7 ParaskP7 mentioned this pull request Mar 21, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants