-
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
Announce cherrypick failure if PR mergability state is blocked #8688
Conversation
I noticed this while doing the mobile deployer chore last week. The PR auto-merge failed due to the mergable-state being blocked but there was no Slack announcement as it wasn't detected as a failure. |
@roryabraham looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
I removed the label. Merging was fine because this workflow change was obviously not going to fail the checks. |
On second thoughts, I don't think it's a good idea to test this as whatever PR I cherry-pick will need to fail one of the checks. It's probably safer to just await the inevitable failure (it happened in 2/4 recent CP's I think) |
I swear I didn't merge this before checks completed ... and that this same thing has happened to me a few times lately where the label was applied in error. Maybe I'm just going crazy... Anyways, I agree that I'm confident enough in this little change we don't need to go out of our way to test it |
🚀 Deployed to staging by @roryabraham in version: 1.1.56-0 🚀
|
🚀 Deployed to production by @francoisl in version: 1.1.56-0 🚀
|
CC @roryabraham
Details
The cherry-pick workflow does not post to the #announce Slack channel if the PR mergable_state is blocked (usually due to the status check not reading as updated). We should start doing this. This suggestion was made https://github.com/Expensify/Expensify/issues/206241#issuecomment-1096577227.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/207221
Tests
PR Review Checklist
Contributor (PR Author) Checklist
N/A - this is purely a workflow change, no other code has been modified
PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followed/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Screenshots
N/A - this is purely a workflow change