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

Check branch-protection of branch where this workflow is run #291

Closed

Conversation

addyess
Copy link
Contributor

@addyess addyess commented May 1, 2024

Issue: #290

Overview

Remove the hard-coded link to the main branch on the charm to support branch protection checks on release branches

Rationale

On push to a main branch, it makes sense to push to latest/edge, so one should check the branch protection rules of main
On push to a release-xxx branch, a charm could publish the xxx/edge and ought to check branch protection rules of that release-xxx branch

Workflow Changes

Look up the active branch based on the workflow event:

Checklist

  • The contributing guide was applied
  • The PR is tagged with appropriate label (urgent, trivial, complex)

@addyess addyess requested a review from a team as a code owner May 1, 2024 13:17
Copy link
Contributor

@cbartz cbartz left a comment

Choose a reason for hiding this comment

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

@addyess You may be interested in reviewing #285 , which will also change the Publish Charm workflow.

@@ -42,6 +42,11 @@ jobs:
branch-up-to-date-check-enabled:
runs-on: ubuntu-22.04
steps:
- env:
BRANCH: ${{ github.base_ref || github.ref }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work if the workflow is triggered by the release event, as github_ref will be equal to a tag (which cannot have branch protection with status check). We should add a short explanation to the docs with the supported events.
https://docs.github.com/en/actions/learn-github-actions/contexts#github-context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbartz

the events i expected to trigger this would have been:

  • push
  • workflow_dispatch

I'm calling this workflow from these events:

on:
  workflow_dispatch:
  push:
    branches:
      - main
      - release-*

An alternative is that this workflow supports a branch input.

I'd REALLY like to be able to merge into a release-1.xx branch, and have the charm published to my 1.xx/edge track. It would make sense this workflow check that the release-1.xx has branch-protection-status enabled the same as main.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but my point is that other users might try to trigger the workflow on the release event, which might fail. Maybe an edge case, but I still think it is worth adding a one-liner in the docs about which workflow events are supported (push, workflow_dispatch, pull_request) to avoid having to troubleshoot this in the future if someone complains.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@addyess , if you could add a small explanation with this in the readme that'd be great

@cbartz
Copy link
Contributor

cbartz commented May 3, 2024

/canonical/self-hosted-runners/run-workflows d158e22

@addyess addyess closed this May 17, 2024
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