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

update kotlin to 1.8.22, replace deprecated kotlin synthetics with jetpack bindings #902

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mesinger
Copy link
Contributor

Updates Kotlin to 1.8.22 and migrates the deprecated kotlin synthetics by jetpack bindings as described here

Done

  • Update Gradle dependencies and plugins
  • Migrate all classes depending on kotlin synthetics to use jetpack
  • run ./update-dependency-pinning.sh
  • connectedDebugAndroidTest 🟥 fails? (is this know, i cannot compare master as that one isn't building on my machine)
  • Run unit tests and manually checking out all screens on Emulator/Device

ℹ️ I did this because I tried to get the project running on my machine and some solution was bumping kotlin.

Copy link

cla-bot bot commented Dec 29, 2023

Thank you for your pull request and welcome to our community! We require contributors to sign our Contributor License Agreement, and we don't seem to have the user @mesinger on file. In order for your code to get reviewed and merged, please explicitly state that you accept the agreement. Alternatively, you can add a commit that adds yourself to https://github.com/grote/Transportr/blob/master/.clabot

@mesinger
Copy link
Contributor Author

mesinger commented Dec 29, 2023

Hey quick side context why i did this PR,

I really like Transportr and there is a specific feature I'd love it to have, so i checked the repo/issues and jadajadajada...
to keep the story short I have read about the slow development speed stuff...

So today I tried to setup my local dev environment and this PR was the result of it. ...

Furthermore I was thinking of the possibility of contributing to this app, despite the fact of knowing the infrequent development the last couple of years and investing time in the onboarding of a dev to a project that lacks invested time anyway is 💀.

As @Altonss and @ialokim you seem to be the most recent contributors, see any sense in further cotributing to Transportr?

My Background:
Full Stack SWE, lot of Java experience and some rusty Kotlin. Did some Android dev at Uni 7 years ago so I know the basics. Some CI/CD (also github-actions ✅) knowledge from my work context (which is mostly Spring-Boot apps stuff).
So I'm confident I could work on most of the code base on my own, but have no clue on things like getting apps to PlayStore/F-Droid stuff like that.

Greetings

EDIT: The feature I'd like to have in Transportr mentioned above, i did a poc here. Feel free to give your feedback to that also.

@Altonss
Copy link
Collaborator

Altonss commented Jan 1, 2024

Hey quick side context why i did this PR,

I really like Transportr and there is a specific feature I'd love it to have, so i checked the repo/issues and jadajadajada... to keep the story short I have read about the slow development speed stuff...

So today I tried to setup my local dev environment and this PR was the result of it. ...

Hello @mesinger,
I'm happy to read your message! Don't hesitate to open an issue about your feature request :)
As for the development setup, I can build Transportr without any issues in Android Studio 🤔

Furthermore I was thinking of the possibility of contributing to this app, despite the fact of knowing the infrequent development the last couple of years and investing time in the onboarding of a dev to a project that lacks invested time anyway is 💀.

As @Altonss and @ialokim you seem to be the most recent contributors, see any sense in further cotributing to Transportr?

My Background: Full Stack SWE, lot of Java experience and some rusty Kotlin. Did some Android dev at Uni 7 years ago so I know the basics. Some CI/CD (also github-actions ✅) knowledge from my work context (which is mostly Spring-Boot apps stuff). So I'm confident I could work on most of the code base on my own, but have no clue on things like getting apps to PlayStore/F-Droid stuff like that.

Greetings

EDIT: The feature I'd like to have in Transportr mentioned above, i did a poc here. Feel free to give your feedback to that also.

Personnaly I find Transportr really useful and would be happy to see this software maintained in the future :) Some things just require basic maintenance, while some new features would require major developments. For now I focus on maintenance, but I'm open for more bigger changes :)

@ialokim
Copy link
Collaborator

ialokim commented Jan 4, 2024

Thank you for this PR and welcome to the small community! :) Indeed it would be great to get more contributors for Transportr, so we are happy to have you here!

This sounds like a fix for #737 and I will happily review it once the next release #888 is out.

@mesinger
Copy link
Contributor Author

mesinger commented Jan 6, 2024

Happy to hear that. Do you have any way to communicate besides github e.g. Discord?

@cla-bot cla-bot bot added the cla-signed ✔️ The Contributor Licence Agreement was signed by all contributors. label Jan 7, 2024
@mesinger
Copy link
Contributor Author

mesinger commented Feb 6, 2024

ping: please review this pr

Copy link
Collaborator

@Altonss Altonss left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR, sorry for the late review.
Transportr is crashing with this error:

java.lang.NullPointerException: Parameter specified as non-null is null: method androidx.lifecycle.ViewModelProvider.<init>, parameter factory
                                                                                                    	at androidx.lifecycle.ViewModelProvider.<init>(Unknown Source:7)
                                                                                                    	at de.grobox.transportr.trips.search.ProductDialogFragment.onCreateView(ProductDialogFragment.java:80)
                                                                                                    	at androidx.fragment.app.Fragment.performCreateView(Fragment.java:2600)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:881)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManagerImpl.java:1238)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:1303)
                                                                                                    	at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:439)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.executeOps(FragmentManagerImpl.java:2079)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.executeOpsTogether(FragmentManagerImpl.java:1869)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.removeRedundantOperationsAndExecute(FragmentManagerImpl.java:1824)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl.execPendingActions(FragmentManagerImpl.java:1727)
                                                                                                    	at androidx.fragment.app.FragmentManagerImpl$2.run(FragmentManagerImpl.java:150)
                                                                                                    	at android.os.Handler.handleCallback(Handler.java:942)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:99)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7940)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:952)

Copy link
Collaborator

@Altonss Altonss left a comment

Choose a reason for hiding this comment

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

Looks good to me :)
Tested on Android 13, and everything worked fine!
Maybe just to be sure, one could also test if it still works on older android versions (like android 5)

@Altonss
Copy link
Collaborator

Altonss commented Nov 2, 2024

I'll try to finally test this PR on an older device before merging it :)
@mesinger the CI seems to be failing, I don't what's the best solution to fix CI, maybe rebasing this PR on master? 🤔

@Altonss
Copy link
Collaborator

Altonss commented Nov 7, 2024

@mesinger the CI seems to be failing, I don't what's the best solution to fix CI, maybe rebasing this PR on master? 🤔

After a rebase on master, CI is fixed (see #962 ) !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed ✔️ The Contributor Licence Agreement was signed by all contributors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate from Kotlin synthetics to Jetpack view binding
3 participants