-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[No QA] Fix verifyPodfile.sh #71657
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
[No QA] Fix verifyPodfile.sh #71657
Conversation
|
|
488eee6 to
9ee4384
Compare
04e933b to
2fb09c6
Compare
|
@sumo-slonik Anything for me to test here or should I just blindly approve? |
The test steps above have been verified and are working. Could you take a look and see if tests/unit/parseAndCollectJSONsTest.ts looks good to you? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
jjcoffee
left a comment
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!
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 am not very experienced with ruby and find its syntax quite ugly but to the best of my knowledge this looks good
|
@sumo-slonik I think |
Thanks, I'll keep that in mind for the future. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.2.23-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.2.23-3 🚀
|
Explanation of Change
The problem was that we were parsing JSONs incorrectly in the previous script that was responsible for removing invalid JSON files with information about used packages. Because of the parsing error, the script was receiving an empty list of pods, which meant we never detected any changes. I’ve corrected the script now, and it should work properly. After testing, everything seems fine.
Fixed Issues
$ #71687
PROPOSAL: https://swmansion.slack.com/archives/C05LX9D6E07/p1757074711334269
Tests
package.jsonreact-native-walletto0.1.10npm install(but do not runpod install)Podfile.lockis not up to dateOffline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.