Skip to content

Remove Jetifier#56

Merged
ParaskP7 merged 5 commits intotrunkfrom
remove-jetifier
Mar 15, 2022
Merged

Remove Jetifier#56
ParaskP7 merged 5 commits intotrunkfrom
remove-jetifier

Conversation

@0nko
Copy link
Copy Markdown
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)

@0nko 0nko requested review from ParaskP7 and malinajirka March 10, 2022 13:02
@ParaskP7 ParaskP7 self-assigned this Mar 10, 2022
@ParaskP7 ParaskP7 added the enhancement New feature or request label Mar 10, 2022
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

2 participants