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

Migrate from Kotlin synthetics to Jetpack view binding #737

Open
ialokim opened this issue Dec 27, 2020 · 12 comments · May be fixed by #902
Open

Migrate from Kotlin synthetics to Jetpack view binding #737

ialokim opened this issue Dec 27, 2020 · 12 comments · May be fixed by #902
Assignees
Labels
beginner job 🔰 This is a relatively easy task suitable for new contributors. priority 🚨️ This issue will be most probably addressed before the next release.
Milestone

Comments

@ialokim
Copy link
Collaborator

ialokim commented Dec 27, 2020

See https://developer.android.com/topic/libraries/view-binding/migration and https://developer.android.com/topic/libraries/view-binding

@ialokim ialokim added the beginner job 🔰 This is a relatively easy task suitable for new contributors. label Dec 27, 2020
@AcharyaS97
Copy link

I've started on this!

@ialokim ialokim assigned ialokim and AcharyaS97 and unassigned ialokim Dec 30, 2020
@AcharyaS97
Copy link

So I had a question about this. I want to finish this up but having issues with some of the UI pieces (PickTransportNetworkActivity, AboutActivity, AboutFragment, ContributorFragment) that rely on external dependencies for some ui binding. The ui classes in question use some of the mikepenz dependencies:

implementation 'com.mikepenz:aboutlibraries:6.2.0'
implementation "com.mikepenz:fastadapter:$fastadapterVersion"
implementation "com.mikepenz:fastadapter-commons:$fastadapterVersion"
implementation "com.mikepenz:fastadapter-extensions-expandable:5.3.4@aar"

I want to update their versions to the latest because they have been updated to support viewbinding, but I'm a little confused how to do that given the fact the project uses the gradle witness plugin. Would I need to manually update the witness file after updating the dependencies so that the checksum on each updated dependency matches what I see on the versions I update to?

@grote
Copy link
Owner

grote commented Feb 12, 2021

Run this script after bumping the dependencies: https://github.com/grote/Transportr/blob/master/update-dependency-pinning.sh

@ialokim
Copy link
Collaborator Author

ialokim commented Feb 12, 2021

Be aware that Mike Penz' libraries tend to introduce API breaking changes with new versions. So be sure to test thoroughly all parts of the app using these libraries...

@AcharyaS97
Copy link

@grote Great thanks i'll do that.

@ialokim So then should I add automated tests for related pieces as a part of that?

@AcharyaS97
Copy link

Also, where are all the variables defined that are used as placeholders for dependency versions?

@ialokim
Copy link
Collaborator Author

ialokim commented May 21, 2021

Also, where are all the variables defined that are used as placeholders for dependency versions?

Did you mean these?

@AcharyaS97
Copy link

yeah. sorry i haven't forgotten about this. i've been having issues with some of the mikepenz libraries and updating those to use viewbinding.

@ialokim
Copy link
Collaborator Author

ialokim commented May 28, 2021

No problem. Would you want to continue working on it or could you push the changes you already did somewhere so someone else could take over?

@ialokim ialokim added the priority 🚨️ This issue will be most probably addressed before the next release. label May 28, 2021
@AcharyaS97
Copy link

I wanted to continue working on it. When did you want me to have it in by?

@ialokim
Copy link
Collaborator Author

ialokim commented May 29, 2021

We have no deadline. I just wanted to make sure that the work that you had already done would not get lost.

I imagine that updating the mikepenz libraries is a big task. Not sure whether we could do the migration in parts, i.e., leaving the fastadapter library updates apart for now?

@AcharyaS97
Copy link

Sure! I'll start committing what I have so far to a branch in my forked repository and submit and a PR for all of it when I've gotten it done. Any pieces that require bigger changes due to libraries being updated I'll leave out. Will summarize in the PR notes what I've left out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner job 🔰 This is a relatively easy task suitable for new contributors. priority 🚨️ This issue will be most probably addressed before the next release.
Projects
None yet
4 participants