Skip to content
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

Ping ApplauseLeads when there's a Cherry Pick #4212

Closed
isagoico opened this issue Jul 24, 2021 · 10 comments
Closed

Ping ApplauseLeads when there's a Cherry Pick #4212

isagoico opened this issue Jul 24, 2021 · 10 comments
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2

Comments

@isagoico
Copy link

For the past deploy checklists there has been some cherry pick PRs that need QA. When these are CPd to fix a deploy blocker there's no notification (for us at Applause) to test those PRs and verify if the fix was a pass or not.

It would be great and more effective if there's a notification tagging the Applause Leads when there's a CP that needs QA.

@isagoico isagoico added Daily KSv2 Improvement Item broken or needs improvement. labels Jul 24, 2021
@iwiznia
Copy link
Contributor

iwiznia commented Jul 26, 2021

How would we know if the CP needs QA or not? I can't think of an easy way to automate this really...

@isagoico
Copy link
Author

Mmm if the CP is fixing a deploy blocker we should probably retest if the issue is not reproducible so it can be checked off the checklist. Also, I'm almost sure the CPs made recently included some QA steps, let me look for one.

@MelvinBot
Copy link

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@isagoico
Copy link
Author

Here's an example of a CP that has QA #4020

Also, forgot to tag you guys before :) Can you give a look at this proposal CC @roryabraham @AndrewGable

@thienlnam
Copy link
Contributor

Seems like we can just use the CP Staging tag that already exists to ping ApplauseLeads for when PRs get merged with this?

@thienlnam thienlnam added Weekly KSv2 and removed Daily KSv2 labels Jul 26, 2021
@roryabraham
Copy link
Contributor

Seems like we can just use the CP Staging tag that already exists to ping ApplauseLeads for when PRs get merged with this?

Not quite, because PRs can be manually CP'd after-the-fact in the event the author forgot to give the PR the CP Staging label when it was merged. Also, it's more useful to ping them once the CP is completed, rather than when the PR is merged.

So the solution for this should really be implemented in the markPullRequestsAsDeployed GH Action. You can see here is where we determine whether a deployed PR was CP'd or deployed normally. In the event it was CP'd, we could also tag @Expensify/applauseleads in the deploy comment.

@roryabraham
Copy link
Contributor

And to @iwiznia's point, we could skip tagging @Expensify/applauseleads if the PR title includes the string No QA (case insensitive). Worst case scenario, they are tagged and see that there are no QA steps for that PR, which should be NBD.

@thienlnam thienlnam added the Internal Requires API changes or must be handled by Expensify staff label Jul 30, 2021
@thienlnam
Copy link
Contributor

Going to keep myself assigned to work on this internal issue and try to complete it later this week / early next

@thienlnam
Copy link
Contributor

Got a little sidetracked with Chronos stuff, but still working on this. I got some instructions on how to set up octokit form @roryabraham and need to play around with that to check some parameters

@MelvinBot MelvinBot removed the Overdue label Aug 17, 2021
@thienlnam thienlnam added the Reviewing Has a PR in review label Aug 18, 2021
@botify botify closed this as completed Aug 20, 2021
@botify
Copy link

botify commented Aug 30, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants