Utilize GitHub Actions to check for SEMVER impact label

[KevinEyo1]

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

2140

What is the area that this feature belongs to?

DevOps

Is your feature request related to a problem? Please describe.

It can be easy to forget to label PRs with SEMVER impact labels when merging.

Describe the solution you'd like

Check PR body description to ensure SEMVER impact is ticked and label the PR with the appropriate label amongst r.Major, r.Minor and r.Patch

Describe alternatives you've considered

Prevent merging if not labelled or fail the job if not labelled. Automate labelling.

Additional context

No response

[yucheng11122017]

An extension to this as discussed today by @KevinEyo1 and I is to remind mergers to tick if it is missing.

Unlike the missing commit message PR, because this step only occurs at the end, a reminder might be more appropriate than failing a status check. This is open for discussion though!

[KevinEyo1]

For preventing merge if label is missing, I'm thinking of having a "status" (implemented using status or something else), that will remain failed but never checked (so checks won't have to be done until the end). The check will only happen on label trigger (to avoid multiple checks), that will check whether one of the 3 labels have been added, then pass the "status" and allow merging.

Also, as far as adding labels, an alternative to the current implementation of adding it at the end would be on pr edited trigger, immediately add the label to the PR, which will fit better for the on label method above.

[kaixin-hc]

Are there any downsides to using the on PR edited trigger?

I like the idea of a reminder rather than a block (the failed test at the bottom might have the side effect of getting us used to seeing a red X when merging PRs, which I don't like the idea of. seeing a red X should be not the norm for PRs that are ready to merge, or it'll stop being as good a heuristic. and having to manually run the test after ticking the box is not something i'm in favour of)

[KevinEyo1]

Hi @kaixin-hc, it is currently implemented on PR merge since the labelling happens at the end, and on PR edit would keep running the job even if you are just editing the description or adding new commits. Hence I think on PR merge trigger would be better if we don't consider preventing the merge.

[tlylt]

on PR edit would keep running the job even if you are just editing the description or adding new commits

What about using on PR approved?

name: Pull Request Review Action

on:
  pull_request_review:
    types: [submitted]

jobs:
  build:
    runs-on: ubuntu-latest
    if: github.event.review.state == 'approved'
    steps:
      - uses: actions/checkout@v2

      - name: Run some action
        run: echo "Pull request was approved!"

[yucheng11122017]

on PR edit would keep running the job even if you are just editing the description or adding new commits

What about using on PR approved?

name: Pull Request Review Action

on:
  pull_request_review:
    types: [submitted]

jobs:
  build:
    runs-on: ubuntu-latest
    if: github.event.review.state == 'approved'
    steps:
      - uses: actions/checkout@v2

      - name: Run some action
        run: echo "Pull request was approved!"

yeah I think this makes the most sense! Before the PR is merged in

@KevinEyo1 could you implement the changes? :)

[KevinEyo1]

Sorry for the late reply, I will work on it right now

[KevinEyo1]

Due to security reasons, for permissions given to GITHUB_TOKEN, the maximum access for pull requests from public forked repositories is restricted to only read, so it is not possible to add labels since there is no write access. GitHub introduced the pull_request_target event that will bypass this restriction and give the token write access. Pros:

• It allows labelling of PR
• Increased security as base branch workflows can be trusted, protecting job from running modified and malicious workflows

Cons:

• It can only run on pull_request events and not pull_request_review events which means need to run on PR merge rather than on PR approved.
• This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This could lead to security vulnerabilities if scripts run are not properly checked for malicious code. Can be aided by seeking approval before running the job, refer to change repo settings

Alternative implementations Workaround Pros: this can allow for still triggering on PR approved Cons: immensely complicates the workflow

Personal Access Token (PAT) Create a PAT with the necessary permissions and add it to your repository's secrets. Then, modify the workflow to use this secret instead of the GITHUB_TOKEN. Pros: this can allow for still triggering on PR approved Cons: exposes your repository to risks if the forked code can access the token

[tlylt]

Due to security reasons, for permissions given to GITHUB_TOKEN, the maximum access for pull requests from public forked repositories is restricted to only read, so it is not possible to add labels since there is no write access. GitHub introduced the pull_request_target event that will bypass this restriction and give the token write access.

Thanks @KevinEyo1 for the investigation. Yes this is indeed a problem and we in fact encountered this when trying to implement pr-preview workflows. See https://github.com/MarkBind/markbind/issues/1466#issuecomment-1064043315 and https://github.com/MarkBind/markbind-action/tree/master/.github/workflows

As for the options you provided, I think we can either go with former (though more complicated with two separate scripts, it should be doable) or with the previous implementation on PR merge.