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

Show a warning comment for PRs targeting the wrong base branch #412

Merged
merged 4 commits into from
Oct 23, 2021

Conversation

DasSkelett
Copy link
Member

Motivation

I've accidentally submitted a pull request targeting master at #410, and we didn't notice it until after the merge (HebaruSan was wondering why it didn't deploy on alpha).
We had to force-push the commits away from master again and re-do the PR to alpha (#411).

GitHub automatically chooses the master branch as it is our default branch.
We want to keep it that way so technology-interested users of SpaceDock that want to look at our source code can find it in the state as it is deployed right now.

On Discord we got the idea to add a workflow that posts a warning comment on a pull request that is not targeting alpha.

Changes

A new branch-warning.yml workflow file is added.
It triggers on pull_request_target: opened, which is equivalent to pull_request except that it runs in the context of the base branch and not the source branch (or an automatically created merge commit thereof).
This seems to be the recommended way for workflows that do not need to interact with the code changes of the source branch, as it has some security benefits (and I guess minimally reduced load on GitHub's servers as it doesn't need to create a merge commit).

It checks for the target and source branch of the PR, and executes if the PR goes to master/beta and does not come from beta/alpha.
I used the label property of the event payload, as it contains the username of the repo owners, so to make sure the warning triggers if someone opens up a PR from their own, changed alpha/beta branch.

Helpful docs links:

Test run

See DasSkelett#2 and DasSkelett#3

I had to do change the trigger to pull_request as otherwise the workflow wouldn't have run until it had been merged.

example comment

While I was on it, I realized that we do not mention our branching strategy anywhere in the README or CONTRIBUTING.md, only the "Development Guide" wiki page. Now CONTRIBUTING.md links to the "Development Guide" wiki page, and README.md links to CONTRIBUTING.md.
Still two clicks away, but better than before.
Additionally I've reworked CONTRIBUTING.md to make it more inline with our current setup.

Copy link
Contributor

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

Great! One minor slip-up in a PR and now we'll have a guard-rail to stabilize our work flow.
💯

@HebaruSan HebaruSan merged commit 0755ae0 into KSP-SpaceDock:alpha Oct 23, 2021
@DasSkelett DasSkelett deleted the feature/branch-warning branch October 24, 2021 15:32
@HebaruSan HebaruSan changed the title Show a warning comment for PRs targeting the wrong base branch Show a warning comment for PRs targeting the wrong base branch Apr 25, 2022
@HebaruSan HebaruSan mentioned this pull request Apr 25, 2022
@HebaruSan HebaruSan mentioned this pull request Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants