Skip to content

[No QA] Change createNewVersion back to dispatchable workflow#9873

Closed
roryabraham wants to merge 9 commits into
mainfrom
Rory-RevertCreateNewVersionCallableWorkflow
Closed

[No QA] Change createNewVersion back to dispatchable workflow#9873
roryabraham wants to merge 9 commits into
mainfrom
Rory-RevertCreateNewVersionCallableWorkflow

Conversation

@roryabraham

Copy link
Copy Markdown
Contributor

Details

This is a partial revert of #9812, which didn't work due to a GitHub Actions bug, which can be tracked in a few places:

I've optimistically curated the revert to include some best practices updates, reduce the diff, and see if this bug we're seeing is related to the use of a callable action, as I suspect. This should hopefully make it easier to un-revert once the bug is fixed.

Fixed Issues

$ n/a – broken deploys

Tests

  1. Merge this PR
  2. Verify that a staging deploy happens, as expected.

@roryabraham roryabraham requested a review from a team as a code owner July 13, 2022 01:30
@roryabraham roryabraham self-assigned this Jul 13, 2022
@melvin-bot melvin-bot Bot requested review from sketchydroide and removed request for a team July 13, 2022 01:30
@roryabraham roryabraham requested a review from AndrewGable July 13, 2022 01:43
@francoisl

Copy link
Copy Markdown
Contributor

Verify that a staging deploy happens, as expected.

Do we need to apply the label CP Staging or is there no checklist atm?

@roryabraham

Copy link
Copy Markdown
Contributor Author

Do we need to apply the label CP Staging or is there no checklist atm?

Checklist is currently unlocked, so no need for the CP Staging label

- name: Checkout main branch
uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f
with:
ref: main

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we no longer need token: ${{ secrets.OS_BOTIFY_TOKEN }} for this step?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope. Never did 🙂

uses: Expensify/App/.github/actions/javascript/triggerWorkflowAndWait@main
with:
GITHUB_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }}
WORKFLOW: createNewVersion.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see that SEMVER_LEVEL is still an input of createNewVersion.yml. To clarify, it's fine not to pass it because it's required: false, right?

@roryabraham roryabraham Jul 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep! Making that parameter optional is one of the changes I've decided to optimistically keep from the other PR

Comment thread .github/workflows/finishReleaseCycle.yml Outdated

@francoisl francoisl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM but I'm not an expert at GH actions, I'll defer to @AndrewGable and @sketchydroide for the merge in case I missed something.

@AndrewGable

Copy link
Copy Markdown
Contributor

Going to let @roryabraham merge

@roryabraham

Copy link
Copy Markdown
Contributor Author

Not going to merge, because I think I know what's wrong

@roryabraham

Copy link
Copy Markdown
Contributor Author

Follow-up: #9879

@roryabraham

roryabraham commented Jul 13, 2022

Copy link
Copy Markdown
Contributor Author

We'll see if that works, and if it does then I'll close this.

@roryabraham roryabraham deleted the Rory-RevertCreateNewVersionCallableWorkflow branch January 29, 2025 16:45
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