-
Notifications
You must be signed in to change notification settings - Fork 3k
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 flaky pod installs #46316
Fix flaky pod installs #46316
Conversation
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
npm has a |
Validate GHA is failing on main, so I created a separate PR to fix it: #46356 |
@ Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@AndrewGable and I chatted about this and decided we'll go with a simpler solution. This PR helped uncover the root cause as partial/stale cache matches with the |
@roryabraham is this ready for a review now after the change in course? thanks! |
yes, this is ready for review. our attempt at a simpler solution didn't work (example) |
We still saw the hermes error on the build for RN0.74 with @MrRefactor so trying to run the adhoc build with this branch https://github.com/Expensify/App/actions/runs/10215905748 |
Running the AdHoc build from this branch isn't a real test, because it will check out the code from the RN upgrade branch and then the script won't be there |
Yeah I see now from the run :/ |
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.
Changes look good to me although I am not real bash expert
Co-authored-by: Vit Horacek <[email protected]>
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
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.
@roryabraham sorry for the delay, now it seems we got some conflicts
# Conflicts: # workflow_tests/assertions/platformDeployAssertions.ts # workflow_tests/assertions/testBuildAssertions.ts
conflicts resolved |
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!
β This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
FYI I believe this was deployed to prod yesterday, from this checklist - #47219 |
I mean the fix was kinda obvious, but I think we should update the documentation maybe? Or is yq a tool that everyone has installed and my env is just weird? |
Well the script is kind of self-documenting with what you need to do, but if you want to open a documentation PR I'd be happy to review that ππΌ idk how standard |
Details
I believe this PR will fix the π΄ββ οΈ dreaded π΄ββ οΈ pod install error:
Rather than writing up a duplicate explanation, I'll simply refer to the code comment.
Fixed Issues
$ n/a
Tests
ios/Pods/Local Podspecs/hermes-engine.podspec.json
and adjust the version to0.74.3
.cd ios && bundle exec pod install
, verify that it fails with the dreaded errorscripts/pod-install.sh
, verify that it succeeds.Can't test with AdHoc builds because even though we can trigger a workflow from this branch, when we run checkout the script isn't present.
Offline tests
None
QA Steps
None.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop