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

build(deps)!: JDK11 and SDK 31+ are required #1552

Merged
merged 4 commits into from
Jun 3, 2022

Conversation

mikehardy
Copy link
Contributor

@mikehardy mikehardy commented Feb 11, 2022

Summary:

The template now inits a compileSdk/targetSdk 31 project,
JDK8 cannot compile reliably against SDK31, so bump JDK as well

I will mark this as draft, it should get a few more commits:

Test Plan:

seems to work for me locally?

Each commit is separate and useful, they should be removed commit by commit to make it all crystal clear

☠️ : ⚠️ the ! in the commit message is a good idea or not, to drive a breaking change semver? ⚠️ ☠️ :

@mikehardy mikehardy marked this pull request as draft February 11, 2022 18:38
@mikehardy
Copy link
Contributor Author

This should be extended to be react-native version specific, it should only fail on JDK11 if react-native is >= 0.68

@grabbou
Copy link
Member

grabbou commented Feb 25, 2022

@mikehardy any update on this PR as it's still marked as draft?

@mikehardy
Copy link
Contributor Author

No progress yet, hoping for a timeslice fir CLI work in a day or two but meeting a contract work deadline first. I'll never take offense if someone just runs with anything I've started though, as long as like you did they drop me a note 😁

Next step here is to personally learn how to access what version of react-native is in play, and start doing checks that provide different guidance based on different rn version

@mikehardy mikehardy marked this pull request as ready for review June 3, 2022 15:36
@mikehardy
Copy link
Contributor Author

@grabbou forgot I still had this one out here is draft. Was in CLI repo just looking to verify something (I needed to toss someone the link to PR removing react-native link functionality and now you got me for two PRs ;-)

@mikehardy
Copy link
Contributor Author

e2e is going to fail until #1620 is merged because it has the github actions bump to JDK 11

// Installing 29 as well so Android Studio does not complain on first boot
const componentsToInstall = [
'platform-tools',
'build-tools;29.0.3',
'platforms;android-29',
'build-tools;31.0.0',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

verified this matches template in react-native repo right now

@mikehardy
Copy link
Contributor Author

I forgot to update tests, my apologies. In progress.

The template now inits a compileSdk/targetSdk 31 project,
JDK8 cannot compile reliably against SDK31, so bump JDK as well
as documented in reactnative.dev, fixes java 8 build fail

(cherry picked from commit d1b8d56)
@mikehardy
Copy link
Contributor Author

fixed tests, cherry-picked the JDK11 CI fix from #1620,

I can rebase that CI JDK11 fix out and re-push or mash the commits around however as needed depending on PR merge order / preference here

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thank you!

@thymikee thymikee merged commit 361089c into master Jun 3, 2022
@thymikee thymikee deleted the doctor-versions-JDK-SDK branch June 3, 2022 17:39
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.

4 participants