-
Notifications
You must be signed in to change notification settings - Fork 906
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
Conversation
This should be extended to be react-native version specific, it should only fail on JDK11 if react-native is >= 0.68 |
@mikehardy any update on this PR as it's still marked as draft? |
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 |
a665971
to
7b0e07f
Compare
@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 |
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', |
There was a problem hiding this comment.
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
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)
7b0e07f
to
6159093
Compare
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
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?