-
Notifications
You must be signed in to change notification settings - Fork 585
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
Fix Install test for RN >= 0.73 #6189
Conversation
ba950ea
to
70161c9
Compare
b2c0d9d
to
aca6331
Compare
386bcb0
to
ba8c5ef
Compare
ba8c5ef
to
70922a9
Compare
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.
Thanks for jumping deep into this 🐇 🕳️ !
|
||
// Ensure metro is really dead | ||
// If this is removed, it's highly likely the android emulator runner will not shut down correctly | ||
const METRO_PORT = 8081; |
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.
If Metro's port was to change for some reason, can this be loaded dynamically rather than hardcoded?
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.
We could, but I think it's not worth the trouble until it actually happens.
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.
I've also wondered if my workaround here could be substituted by sending a stricter exit code to the .exit()
method above.
What, How & Why?
Tests were failing for the newest React Native. The following updates have been made in this PR:
☑️ ToDos
Compatibility
label is updated or copied from previous entryCOMPATIBILITY.md
package.json
s (if updating internal packages)Breaking
label has been applied or is not necessary