Generate a single commit status

[joao-paulo-parity]

In e.g. https://github.com/joao-paulo-parity/prcr-playground/pull/1 it is demonstrated that the current implementation generates two commit statuses, when only 1 of them is actually needed.

[sergejparity]

@joao-paulo-parity unfortunately this is a "feature" of GitHub Actions. If a workflow is triggered by multiple events it will create separate status for every event. That's why this action pull_request always has status of success even if special review is required and following it artificial status check PR Custom Review Status always has failure state. Because at this stage it is not possible to tell whether required approvals received. I made a review of your demo PR from your post above and now it has 3 statuses

[joao-paulo-parity]

I thought it would be possible to overcome this by creating a custom status. Personally I have no problem with this tool creating multiple statuses. I created this issue from @TriplEight 's feedback. @TriplEight do you still think this would be a good idea? I can't see a problem with multiple statuses since we'll only require one of them anyways (https://github.com/paritytech/pr-custom-review/tree/2c4afa8463f9d96d17a73cc7071a9aafe42ce5a4#github-repository-configuration--).

[joao-paulo-parity]

One problem with multiple commit statuses is that I did not find a way to clear lingering failed statuses without pushing a new commit. e.g. the current situation at https://github.com/tripleightech/pr-custom-review-test/pull/8

Reproduction steps:

1. Open a PR with no reviews
2. Wait for PR Custom Review Status / build (pull_request) to fail
3. Approve the PR
4. PR Custom Review Status / build (pull_request_review) will pass, but PR Custom Review Status / build (pull_request) is still failing

[sergejparity]

Experienced exactly the same problem. GitHub does not offer easy way to re-trigger connected events. When examined this problem, I found couple of solutions how to do this, but they were over complicated and super fragile.

My solution was to force pull_request status to be always success. Actually you can see this logic reflected on High level flow chart And that's why I'm using additional independent, artificially created status as the only required.

[joao-paulo-parity]

My solution was to force pull_request status to be always success

That works. I'll implement this logic in #27.

[joao-paulo-parity]

Maybe I'm pointing out the obvious but it's perfectly fine to move this functionality to Gitlab CI. Doing so would sidestep the problem altogether since vanity-service only generates one status per check. We could quite simply use the official Node.js LTS image for that CI job.

Instead of passing the GitHub token through the action input, we'd use a vault secret like it's done for pipeline-scripts: https://github.com/paritytech/substrate/blob/04c7c6abef1e0c6b4126427159090a44f3221333/.gitlab-ci.yml#L562.

[TriplEight]

@TriplEight do you still think this would be a good idea?

Nah, now it's clear that it's better to go with the current design, maybe just name the jobs in an explanatory way.

[TriplEight]

Maybe I'm pointing out the obvious but it's perfectly fine to move this functionality to Gitlab CI.

I did not account for moving this tool to be executed in GitLab simply because GH action can be triggered on ...

on:                              # The events which will trigger the action.
  pull_request:                  # A "pull_request" event of selected types will trigger the action.
    branches:                    # Action will be triggered if a PR is made to following branches.
      - main
      - master
    types:                       # Types of "pull_request" event which will trigger the action.
      - opened                   # Default event - PR is created.
      - reopened                 # Default event - closed PR is reopened.
      - synchronize              # Default event - PR is changed.
      - review_request_removed   # Requested reviewer removed from PR. Action will re-request its review if it's required.
  pull_request_review:           # PR review received. Action will check whether PR meets required review rules.

... a much wider collection of the events. This, in my head, would have to help preventing unneeded manual re-triggering of the review-checking job. Making it automated and, in turn, used in further automation such as auto merge.

[TriplEight]

is it possible to retrigger such a test with some other event? We'll use ready to merge label in the pre-merge pipelines tool, why not make use of it here with i.e. label? So in the case I need to (re-)trigger the reviews check one could (re-)add that label. Maybe a bit clumsy, but couldn't come up with something better yet.

[joao-paulo-parity]

According to our Matrix discussions, we decided to not put it in Gitlab CI due to the move overall increasing complexity compared to the current approach. By using Gitlab we'd have to trigger the job manually through the Gitlab API instead of the current GitHub action approach which is simpler.

Continues in #40.