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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update various libraries and migrate to JDK 17 #343

Merged
merged 9 commits into from Jul 7, 2023

Conversation

Kurt-Bonatz
Copy link
Contributor

@Kurt-Bonatz Kurt-Bonatz commented Jun 29, 2023

馃搶 References

馃帺 What is the goal?

Android Gradle Plugin 8 requires that apps use JDK 17. Currently, Shot stands as a blocker to using JDK 17 on projects that rely on it.

How is it being implemented?

  • Simplify and centralize the codebase's various libraries with Gradle's version catalog. This should make it easier to understand versions being used as well updating them in the future.
  • Update the majority of the libraries and versions used in the codebase:
    • Scala: 2.12 -> 2.13
      • Includes most scala libraries, notably scrimage to 4.0.37 to resolve (Upgrade Scrimage to latest version聽#339)
      • Resolves some warnings around Scala 3 syntax, but mocking libraries have yet to adopt Scala 3 so I kept it to 2.13
    • Kotlin: 1.4.32 (and others) -> 1.8.20
    • AGP: 7.1 (and others) -> 7.4.2, 8.0.2 (in the compose sample project for verification)
    • Java: 1.8 -> 11
    • JDK: 11 -> 17 (Note: this will require all consumers to also use JDK 17+)
    • Gradle: 7.0.1 (and others) -> 7.6.1, 8.1.1 (in the compose sample project for verification)
    • Android compile/targetSdk: 28 -> 33
    • Most Android libraries updated to a recent stable release
    • Mockito: 2/3 -> 5
      • This also adopts the official mockito kotlin library
    • Removed the Kotlin Android extensions
    • Removed JCenter where possible

How can it be tested?

All steps completed below with local maven published version on a Mac M1 using JDK 17 (#287)

  • AGP 8 & Gradle 8:
    • The shot-consumer-composer sample project now targets these latest stable versions and executeScreenshotTests runs as expected
  • AGP 7 & Gradle 8:
    • A Production app's screenshot tests produce the same results as the production 5.14.1 version of shot
  • AGP 7 & Gradle 7:
    • The sample projects run the various executeScreenshotTests as expected and produce the same results as the production 5.14.1 version of shot
  • Checks and tests succeed:
    • ./gradlew clean checkScalaFmtAll ktlintCheck test :shot-android:connectedCheck passes locally
  • Passing CI:

@Kurt-Bonatz
Copy link
Contributor Author

I've had a number of issues getting the sample projects on master completing the executeScreenshotTests tasks. My changes produce the same results as 5.14.1 on the updated sample projects, but several of the screenshot sizes are different than what are on master. So it's likely that CI might fail and I might need to re-record the screenshots as some library updates like the bump to Material could have changed some of the Android views.

@amatsegor
Copy link

amatsegor commented Jul 3, 2023

That looks like a lifesaver, thank you for your big work!
I think this issue can be also mentioned as a reference

@pedrovgs pedrovgs self-requested a review July 7, 2023 07:41
@pedrovgs
Copy link
Owner

pedrovgs commented Jul 7, 2023

Thank you so much @Kurt-Bonatz, let me take a look at the PR. It's quite big, but as the CI is passing I think we are good to go unless we find any major blocker 馃槂

Copy link
Owner

@pedrovgs pedrovgs left a comment

Choose a reason for hiding this comment

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

This is just perfect @Kurt-Bonatz Thanks for your contribution, I really appreciate it 馃槂 I'm merging this PR right now and releasing Shot 6.0.0. Please, test after this release so we can ensure everything is working as expected 馃槂

@pedrovgs pedrovgs merged commit 2dec201 into pedrovgs:master Jul 7, 2023
2 checks passed
This was referenced Jul 7, 2023
@pedrovgs
Copy link
Owner

pedrovgs commented Jul 7, 2023

Version 6.0.0 released, please @Kurt-Bonatz check if everything is working as expected from your end and let me know if there is something broken. Thanks 馃槂

@Kurt-Bonatz
Copy link
Contributor Author

@pedrovgs Apologies for the lengthy change 馃槄, and thanks for taking the time to review it! Thus far we haven't had any issues with the new version, though I did re-record our screenshots for good measure.

Also things worked out perfectly since you actually built and published the artifact with JDK 11 (don't worry things still work as expected on JDK 17), which means we could drop the new version in without also having to complete the JDK 17 migration on CI or locally. It actually simplified things quite a bit for us!

@pedrovgs
Copy link
Owner

That's why I decided to use 6.0.0 as the new version. The changes will force users to update some of their dependencies and maybe the JDK they use 馃槂 Thanks for your contribution @Kurt-Bonatz I really appreciate it!

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.

None yet

3 participants