add checks for PRs to release branchs

[achou11]

Towards #4

Adds the following checks:

1. Makes sure any PR to a release/* branch is an RC branch (i.e. starts with rc/)

2. Makes sure the version introduced by the RC PRs:

• is not a pre-release
• matches the branch name

Still missing:

• check to make sure the version bump is correct EDIT: to be done as a follow-up

Question: should we rename the workflow file? Introducing the job that has these checks makes the name Check Release Branch a little unclear to me. EDIT: I've renamed the worfklow file to be more explicit (see 8ca1c89)

[achou11]

there might be a better way of doing this

[gmaclennan]

yes, github.head_ref (doesn't require checkout)

[achou11]

does parsing the version (used in other steps) need checkout though?

[gmaclennan]

yes

[gmaclennan]

Is fine as-is, but left some suggestions if you want to save some character :)

[gmaclennan]

Could be worth asserting PR is from an RC branch with a separate job to keep it simple:

    check_source_branch:
        name: Check Source Branch
        runs-on: ubuntu-latest

        steps:
            - name: Check Source Branch
              if: ${{ !startsWith(github.head_ref, 'rc/') }}
              run: exit 1

[achou11]

addressed via 2ad1832

[gmaclennan]

yes, github.head_ref (doesn't require checkout)

[gmaclennan]

This works, and briefer isn't necessarily better, but this would also work I think:

Suggested change

	env:
	RELEASE_VERSION: ${{ steps.version.outputs.releaseVersion }}
	run: |
	set -x
	echo $RELEASE_VERSION
	if [ $RELEASE_VERSION = *-pre ]; then
	echo "Release version cannot be pre-release"
	exit 1
	fi
	if: endsWith(steps.version.outputs.releaseVersion, '-pre')
	run: exit 1

[achou11]

yeah a matter of portability vs readability. think for this readability is probably preferable so applying this suggestion seems reasonable

[achou11]

addressed via 2ad1832

[gmaclennan]

Similar to above, if we want to lean into github actions syntax

Suggested change

	env:
	PR_BRANCH_NAME: ${{ steps.pr_branch_name.outputs.result }}
	RELEASE_SHORT: ${{ steps.version.outputs.releaseShort }}
	run: |
	set -x
	echo $PR_BRANCH_NAME
	echo $RELEASE_SHORT
	if [ $PR_BRANCH_NAME != "rc/v$RELEASE_SHORT" ]; then
	echo "Release version does not match RC branch"
	exit 1
	fi
	if: format('rc/{0}', steps.version.outputs.release) != github.head_ref
	run: exit 1

[achou11]

addressed via 2ad1832

[achou11]

[ ] check to make sure the version bump is correct

@gmaclennan any thoughts on whether I should address this in this PR or do in a follow up?

Separately, is it necessary to add a check to make sure the RC branch name is aligned with the release branch name? I'm leaning towards no because of the rulesets making a situation where they're misaligned unlikely

[gmaclennan]

[ ] check to make sure the version bump is correct

@gmaclennan any thoughts on whether I should address this in this PR or do in a follow up?

This one is a bit trickier so I think can be done (much) later - it's an edge case.

Separately, is it necessary to add a check to make sure the RC branch name is aligned with the release branch name? I'm leaning towards no because of the rulesets making a situation where they're misaligned unlikely

Since the user can't rename either branch or create either branch I think we don't need a check.

[gmaclennan]

Since the user can't rename either branch or create either branch I think we don't need a check.

I mean I guess someone could edit a PR and change the base of the PR to a different release/** branch... So maybe a check to add to the backlog, but this would be covered by checking the version bump as above. Hopefully no-one would do that anyway!

[achou11]

maybe i'm missing something, but what is steps.version.outputs.release? I don't see any reference to it in the parse-versions.yml file?

Maybe i'm supposed to use releaseShort instead?

[achou11]

using my intuition and adjusted via 685dff8

[gmaclennan]

Looks solid to me!