-
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
[No QA]Link to GitHub release in Slack message #11670
Conversation
Just need to find a place to test, then I will open it up for reviews |
Ready for review, tested and it worked! |
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 (left a NAB question just for learning)
@@ -306,7 +306,7 @@ jobs: | |||
channel: '#deployer', | |||
attachments: [{ | |||
color: 'good', | |||
text: `🎉️ Successfully deployed ${process.env.AS_REPO} v${{ env.VERSION }} to ${{ fromJSON(env.SHOULD_DEPLOY_PRODUCTION) && 'production' || 'staging' }} 🎉️`, | |||
text: `🎉️ Successfully deployed ${process.env.AS_REPO} <https://github.com/Expensify/App/releases/tag/${{ env.VERSION }}|${{ env.VERSION }}> to ${{ fromJSON(env.SHOULD_DEPLOY_PRODUCTION) && 'production' || 'staging' }} 🎉️`, |
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.
NAB: This is the first time I see this syntax for links: <link url | link text>
, is it a syntax coming from Slack? or yml?
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.
This is Slack's syntax: https://api.slack.com/reference/surfaces/formatting#linking-urls
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.
Cool! thanks!
PR Reviewer Checklist
|
|
I copied the checklist from here, any idea what I missed? |
4th time lucky I guess 😄 Thanks for implementing this @AndrewGable! |
@Julesssss 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. |
Calm down Melvin, this is not an emergency. All checks had passed. |
[No QA]Link to GitHub release in Slack message
🚀 Deployed to staging by @Julesssss in version: 1.2.15-0 🚀
|
🚀 Deployed to production by @yuwenmemon in version: 1.2.15-3 🚀
|
Details
Adds a link to the GitHub release for the deploy message posted into Slack.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/226803
Tests
I tested this via this test workflow: https://github.com/Andrew-Test-Org/Public-Test-Repo/actions/runs/3236650312/workflow
PR Review Checklist
PR Author Checklist
Note: Not applicable as no App source code has changed, just deploy code
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** 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
No QA
Screenshots
Web
Note: Not applicable as no App source code has changed, just deploy code
Mobile Web - Chrome
Note: Not applicable as no App source code has changed, just deploy code
Mobile Web - Safari
Note: Not applicable as no App source code has changed, just deploy code
Desktop
Note: Not applicable as no App source code has changed, just deploy code
iOS
Note: Not applicable as no App source code has changed, just deploy code
Android
Note: Not applicable as no App source code has changed, just deploy code