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

[No QA] Resolve CP merge conflicts #3350

Merged
merged 6 commits into from
Jun 4, 2021
Merged

Conversation

roryabraham
Copy link
Contributor

Details

More context starting here, but in broad strokes this PR will:

  1. Auto-resolve conflicts with the version-bump PR, accepting the version of package.json, package-lock.json, etc... that contain the higher version (i.e: the branch we're CPing into staging, hence the use of -Xtheirs).
  2. For other merge conflicts, just commit the conflicting files, open a PR, and auto-assign it to an engineer.
  3. Change deploy.yml such that it will deploy to staging when any PR created by OSBotify is merged by anyone, where before it would only deploy to staging when any PR was merged by OSBotify.
    • Note that to accomplish this, I had to point at a forked version of actions-ecosystem/get-merged-pull-request. PR here.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/155229

Tests

  1. Lock the StagingDeployCash
  2. Merge a pull request with an easily identifiable change (no CP Staging label). Call this PR A. As per usual, it should not be deployed or added to the StagingDeployCash. A comment should appear on the PR stating that it will be deployed later.
  3. Merge a pull request with an easily identifiable change with the CP Staging label on the PR. Call this PR B. A new version should be created and merged to main, and the preDeploy workflow should synchronously execute the cherryPick.yml workflow.
  4. A staging deploy should occur, and a comment should appear on PR B stating that it was cherry picked (not just deployed) to staging in the correct version.
  5. Once the staging deploy completes, verify that PR A is not present on staging, but PR B is. This is where the easily-identifiable changes will come in handy.
  6. Verify that PR B was added to the StagingDeployCash, but PR A was not.
  7. Non-authorized actor: Use the GitHub UI to attempt to manually CP PR A to staging. The workflow should not complete. Verify that no staging deploy happens.
  8. @AndrewGable or @roryabraham - Use the GitHub UI to attempt to manually CP PR A to staging. It should work and a staging deploy should occur.
  9. Once the staging deploy completes, verify that PR A is present on staging.
  10. Verify that PR A was added to the StagingDeployCash.
  11. Create revert PRs to revert all the temporary changes. Give each the CP Staging label so the reverts are CP'd to staging. Merge them both to CP them to staging.
  12. Close the StagingDeployCash to run a prod deploy. Verify that the latest staging version is deployed to production.

@roryabraham roryabraham requested a review from AndrewGable June 3, 2021 19:01
@roryabraham roryabraham self-assigned this Jun 3, 2021
@roryabraham roryabraham requested a review from a team as a code owner June 3, 2021 19:01
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team June 3, 2021 19:01
.github/scripts/cherryPick.sh Outdated Show resolved Hide resolved
.github/scripts/cherryPick.sh Outdated Show resolved Hide resolved
run: |
git add -A
git cherry-pick --continue
if [[ "$(./.github/scripts/cherryPick.sh ${{ steps.getCPMergeCommit.outputs.MERGE_COMMIT_SHA }})" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we test this line somehow? It looks like there is a few gotchas that could break this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we didn't test it. I can try to do something similar in Public-Test-Repo to validate this a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some testing here, found one potential issue/simplification with the syntax here.

@roryabraham roryabraham requested a review from AndrewGable June 3, 2021 21:21
ctkochan22
ctkochan22 previously approved these changes Jun 3, 2021
@roryabraham
Copy link
Contributor Author

Updated! All the bash is inlined in the workflow now

Copy link
Contributor

@AndrewGable AndrewGable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Super simple now.

@AndrewGable AndrewGable merged commit cda090f into main Jun 4, 2021
@AndrewGable AndrewGable deleted the Rory-ResolveCPConflicts branch June 4, 2021 20:55
@OSBotify
Copy link
Contributor

OSBotify commented Jun 4, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

OSBotify commented Jun 7, 2021

🚀 Deployed to staging in version: 1.0.63-1🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Jun 8, 2021

🚀 Deployed to production in version: 1.0.64-0🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants