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] Migrate updateProtectedBranch from workflow to composite action #9654

Merged
merged 2 commits into from
Jul 5, 2022

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Jun 30, 2022

Details

This is part of a minor cleanup initiative I've been working on to deprecate some hacky workarounds we had in our GitHub Actions code to GitHub-native functionality, which improves the speed, clarity, and transparency of our CI/CD. It should also help reduce barriers to entry to our GitHub Actions code because it will be following the practices in the GitHub Actions documentation.

In this instance, not having to wait for workflows to spin up or conclude will speed things up pretty substantially.

Fixed Issues

$ (partial) prepping for https://github.com/Expensify/Expensify/issues/195693

Tests

  1. Merge this pull request with the CP Staging label
  2. Verify that it deploys as expected.

@roryabraham roryabraham requested a review from a team as a code owner June 30, 2022 23:01
@roryabraham roryabraham self-assigned this Jun 30, 2022
@melvin-bot melvin-bot bot removed the request for review from a team June 30, 2022 23:02
@github-actions
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@melvin-bot melvin-bot bot requested a review from Luke9389 June 30, 2022 23:02
@@ -0,0 +1,140 @@
name: Update Protected Branch
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewer note: this action is largely the same as the deleted .github/workflows/updateProtectedBranch.yml, so comparing it to that might make it easier to review.


jobs:
updateBranch:
if: github.actor == 'OSBotify'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewer note: This check is not needed anymore. Because it's now an action and not a manually-triggerable workflow, there's no way a non-bot could trigger this action manually, which is why we had this check in the first place.

fi

# Version: 2.3.4
- uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewer note: This checkout is no longer needed because, unlike the workflow, the action runs on the same machine that triggers it. Since the action is loaded from a local URL, we know for sure that if we're in the action we will have already ran a checkout and will have the repo cloned on the runner.

echo "SOURCE_BRANCH=${{ github.event.inputs.SOURCE_BRANCH }}" >> "$GITHUB_ENV"
fi

- uses: Expensify/App/.github/actions/composite/setupGitForOSBotify@main
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I did an experiment and confirmed that you can use nested composite actions

# Version: 3.3.0
uses: umani/changed-files@1d252c611c64289d35243fc37ece7323ea5e93e1
with:
repo-token: ${{ github.token }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I did an experiment and confirmed that, even though secrets are not directly available in an action, the full github context, including github.token is available.

@roryabraham roryabraham changed the title [HOLD][No QA] Migrate updateProtectedBranch from workflow to composite action [No QA] Migrate updateProtectedBranch from workflow to composite action Jun 30, 2022
@roryabraham
Copy link
Contributor Author

Off hold and ready for review 🙂

@roryabraham roryabraham requested a review from a team July 1, 2022 18:24
@roryabraham
Copy link
Contributor Author

Requesting another reviewer since @Luke9389 is OOO

@melvin-bot melvin-bot bot requested review from youssef-lr and removed request for a team July 1, 2022 18:24
@marcochavezf marcochavezf self-requested a review July 5, 2022 23:26
Copy link
Contributor

@marcochavezf marcochavezf left a comment

Choose a reason for hiding this comment

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

LGTM

@roryabraham
Copy link
Contributor Author

Okay, let's test this out 🤞

@roryabraham roryabraham merged commit ab39f7d into main Jul 5, 2022
@roryabraham roryabraham deleted the Rory-UpdateProtectedBranchCompositeAction branch July 5, 2022 23:44
@roryabraham
Copy link
Contributor Author

Monitoring here

@OSBotify
Copy link
Contributor

OSBotify commented Jul 8, 2022

🚀 Deployed to staging by @roryabraham in version: 1.1.81-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.82-5 🚀

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants