[HOLD for payment 2023-09-20] [HOLD for payment 2023-09-18] Manually CPed App PR did not update the checklist

[thienlnam]

We CPed this App PR manually #21601

It was successful, and created a new version #21601 (comment)

However, the checklist #21650 did not update the version or include the blocker

This is bad as it doesn't notify us of the CP and applause does not test the new feature or version

[roryabraham]

Thanks for creating the issue @thienlnam

[roryabraham]

I think I'm going to make this a weekly / probably not work on it until I'm back from OOO next Thursday

[roryabraham]

No update. #21651 will be merged soon and then I'll put up a predesign for the next deploy improvements we want to make

[roryabraham]

No update

[roryabraham]

• Example PR: [CP Staging] Fix/regression unread messages #23729
• Did not get the correct deploy comment: [CP Staging] Fix/regression unread messages #23729 (comment) (should say CP'd to staging)
• CP workflow run: https://github.com/Expensify/App/actions/runs/5693035236
• Appeared in the next checklist: Deploy Checklist: New Expensify 2023-07-27 #23765
• Checklist was created in https://github.com/Expensify/App/actions/runs/5685619602/job/15410896724

[roryabraham]

I verified that this does not appear to be an issue with a lack of complete history by:

• Locally running git pull to get updated (full) history for the repo
• Ran node ./tests/utils/getPullRequestsMergedBetween.mjs '1.3.46-0' '1.3.47-0'
• Opened a node REPL and:
  • Declared an array with the local output of getPullRequestsMergedBetween.mjs
  • Declared an array with the remote output of getPullRequestsMergedBetween.mjs in https://github.com/Expensify/App/actions/runs/5685619602/job/15410896724
  • Used _.isEqual to verify that the arrays are deep-equal

[roryabraham]

Ok, this problem is actually also causing CP'd PRs to appear again on the next checklist too:

• [CP Staging] Fix/regression unread messages #23729 - CP'd in 46-1
• Checklist's final version was 46-0 ❌: Deploy Checklist: New Expensify 2023-07-26 #23657
• This also causes the next checklist to look at the diff between 46-0 and 47-0 which (correctly) includes the cherry-picked PR

[roryabraham]

One of the broken lines we have is here. Since we no longer (always) do a PR for cherry-picks, looking for that merge commit won't work reliably. That explains the bad deploy comment

Another couple potentially bad lines are:

• App/.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed.js

  Line 136 in 1771f2c

  const CPActorMatches = data.message.match(/Merge pull request #\d+ from Expensify\/(.+)-cherry-pick-staging-\d+/);

• App/.github/libs/GitUtils.js

  Line 92 in e24f947

  const match = commit.subject.match(/Merge pull request #(\d+) from (?!Expensify\/.*-cherry-pick-staging)/);

[roryabraham]

So we should fix all those. We currently pass the -x flag to all our git cherry-pick commands which appends (cherry-picked from HASH) to the CP commit messages. So we should be able to match against that instead of Merge pull request #(\d+) from (?!Expensify\/.*-cherry-pick-staging).

All that said, the root cause here is that we update the deploy checklist in preDeploy.yml here, and that line's not getting hit in the cherryPick.yml workflow at all.

We currently to run that job in a few cases:

• After updating staging in the finishReleaseCycle.yml workflow.
• After a PR is merged + deployed (kind of an edge case because it only happens when the checklist is unlocked)
• When a deploy blocker is created

I think we could update it to run whenever:

• A staging deploy (of any kind) completes
• When a deploy blocker is created

and that would address the root cause here

[roryabraham]

Asked for a volunteer to take this over: https://expensify.slack.com/archives/C056AV5V8UX/p1690914225012259

[roryabraham]

Prioritizing comment linking, will follow-up with this after I get that ready

[roryabraham]

My solution to this got a bit big + unruly, so I'm going to break it up into a few incremental PRs that should be easier to review.

First incremental PR is ready for review here: #26297

[roryabraham]

Part 2 ready for review: #26301

---

I'll stop there for now until those parts are reviewed + merged.

[roryabraham]

With #26295, this should now be fixed

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.67-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NO QA] Update checklist for cherry-picked PRs #26295

If no regressions arise, payment will be issued on 2023-09-18. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.68-17 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NO QA] Update checklist for cherry-picked PRs #26295

If no regressions arise, payment will be issued on 2023-09-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty