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.

Sign up for GitHub

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

Jump to bottom

[feature/PI-601] Set branch ref to main if branch has been deleted on workspace delete #401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[feature/PI-601] Set branch ref to main if branch has been deleted on workspace delete #401

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/on-pr-close.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
steps:
- uses: actions/checkout@v4
with:
ref: ${{ env.BRANCH_NAME }}
ref: ${{ env.BRANCH_NAME || '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.

So because the branch gets deleted it needs to run the build in "main". If it's just a regular close of PR this will remain on the branch name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I presume that env.BRANCH_NAME isn't empty (otherwise the step below destroy--redundant-workspaces BRANCH_NAME=origin/${{ env.BRANCH_NAME }} wouldn't work)

think that means that ${{ env.BRANCH_NAME || 'main' }} will never evaluate to main?

Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if the branch doesn't exist, does it fail? if so could use

    if: ${{ failure() }}

to catch a failed checkout, and then checkout main instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So BRANCH_NAME will have a value. As it's coming from the pull request. For definite. That's how destroy--redundant-workspaces BRANCH_NAME=origin/${{ env.BRANCH_NAME }} will still work as in this script the branch name is used to determine the workspace name.

But the point is if the branch doesn't exist as a branch then it will fallback to main.

Apparently this works. But I'm more than happy to concede there is an element of doubt.
There is someone asking for it to be a feature actions/checkout#512 back in 2021 and at the bottom there is someone showing a different option.

- uses: ./.github/actions/make/
with:
command: build
Expand All @@ -41,7 +41,7 @@ jobs:
steps:
- uses: actions/checkout@v4
with:
ref: ${{ env.BRANCH_NAME }}
ref: ${{ env.BRANCH_NAME || 'main' }}
Copy link
Contributor Author

@jameslinnell jameslinnell Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So because the branch gets deleted it needs to run the build in "main". If it's just a regular close of PR this will remain on the branch name.
However this is only building the environment.
On line 49 below it is still using the original branch name to destroy the workspace

fetch-depth: 0
- name: Remove PR workspaces
uses: ./.github/actions/make/
Expand Down
Loading