-
Notifications
You must be signed in to change notification settings - Fork 147
Revert "Disable Integration job on main branch" #957
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
Conversation
|
I'm actually beginning to think the fix might be that react-native-test-app needs to update their own podfile.lock / update their template with react-native-macos 0.66. @tido64 thoughts? |
Can you add
The repo is currently at 0.64. Updating it for 0.66 would break everything else. We need to remove this file regardless because it's not forwards-compatible. I updated the integration script in RNTA, but forgot to do it here as well. |
Thanks! |
The latest CI run fails from the familiar We can probably add that script into RNTA (though I recall RNTA removing all their postinstall scripts recently so they may not want that), but it does make me think of the question of dependencies again. With every RN-macOS release, will this check only pass once we've published a stable version package and RNTA has had a chance to update themselves? If so, should we keep it disabled on |
Sorry for the inconvenience. Ideally, fixes like this should be hidden behind the one post-install script, Edit: microsoft/react-native-test-app#704
I wouldn't be so quick to throw this out. This isn't a required run so it failing doesn't block anyone. If the red-X is annoying, we can probably reduce it to a warning. However, it has caught many real issues and I think it would be better if we could be pro-active rather than reactive about fixing them. |
I didn't notice it was not required, thank you :D. I agree not to throw it out, my apologies :). I wanted to understand the dependency graph better, and having it fail so we can be proactive makes total sense! |
@tido64 Any idea why the "validate manifest" script phase fails? |
It's a new thing I added to make sure that Edit: This should be fixed now. We should also bump Node to a later version. Latest LTS is now 16. |
Filed #975 |
Reverts #947
Closes #948