-
Notifications
You must be signed in to change notification settings - Fork 896
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
Remove hardcoded React Native configuration #767
Conversation
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.
LGTM
@@ -79,8 +105,7 @@ test('should return dependencies from package.json', () => { | |||
|
|||
test('should read a config of a dependency and use it to load other settings', () => { | |||
writeFiles(DIR, { | |||
'node_modules/react-native/package.json': '{}', | |||
'node_modules/react-native-test/package.json': '{}', |
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.
why this is removed?
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.
Accident, but I am surprised that the tests are passing.
Tests are failing on Windows. I will need a Windows machine to debug why the mocking logic doesn't work. If there's anybody with Windows that's willing to give it a go, let me know! |
@thecodrr would you like to help us debug this? |
Yup. On it. |
114fedc
to
307f50b
Compare
Summary:
Historically, React Native used to be tightly coupled with the CLI, support for Android and iOS was hardcoded. Since 0.60, we have made React Native ship with its own configuration, specifying Android and iOS platforms just like React Native Windows.
This PR removes "dead-code" that was only needed during the transition period. It wasn't used and called in the 0.60.
I have also removed some obsolete tests that were no longer needed.