-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[NoQA] Fix failing lockStagingDeploys job #39480
[NoQA] Fix failing lockStagingDeploys job #39480
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.
Couple comments
.github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys.ts
Outdated
Show resolved
Hide resolved
@mountiny PR updated! |
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.
I see now that the GITHUB_BASE_URL_REGEX
is only used in the other consts so good to go ahead I think
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. |
🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.61-0 🚀
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.61-0 🚀
|
🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.61-8 🚀
|
Details
Convert GithubUtils.ts to a legit ES module (remove module.exports), fix related files and tests, extract constants to a separate file.
While trying to debug the related issue I noticed some possible issues caused by multiple exports in
GithubUtils.ts
, we hadmodule.exports
andexport default
at the same time.awaitStagingDeploys.ts
imported the utils withrequire()
and then tried to tried to accessPOLL_RATE
which was only exported withexport {}
so it was undefined which very likely was the reason for that crash.I started by removing
module.exports
fromGithubUtils.ts
and fixing the related files that accessed it withmodule.exports
. By doing that I found out thatawaitStagingDeploysTest.ts
was getting stuck forever on some promise. I moved constants fromGithubUtils.ts
to a separate file and mockedPOLL_RATE
inside that test file and the tests started working again.Fixed Issues
$ #39066
PROPOSAL: N/A
Tests
Offline tests
QA Steps
Lets monitor the workflows will work as expected
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