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

Working label checker in Go #13

Merged
merged 1 commit into from
May 3, 2020
Merged

Working label checker in Go #13

merged 1 commit into from
May 3, 2020

Conversation

johnboyes
Copy link
Contributor

@johnboyes johnboyes commented May 3, 2020

Written in Go, using Mage: https://magefile.org/

Code is loosely based on this example:
https://www.sethvargo.com/writing-github-actions-in-go/

Checks for exactly one of the specified labels, and fails if there are
none or more than one.

Future enhanced functionality could be to allow the client application
to also specify a set of labels none of which should be present, and
a set all of which should be present.

Two noteworthy things to improve in future commits:

  1. There is a bug whereby a PR can show that the check has passed, but
    still not allow the pull request to be merged. This is because GitHub
    Actions create a separate new instance of the check for some different
    event types, e.g. the labeled event creates a new check instance
    separate from the push event checks. So if we, for instance:

  2. push a commit when there is no required label

  3. add the required label
    then the lableled instance of the check will pass, but the push will
    still show the initial failure.
    The proposed fix for this (in a future commit) is to retrieve the GitHub
    labels for the pull request via the GitHub API, instead of using the
    labels in the GitHub Action's event JSON (which is a frozen snapshot of
    the labels at the time of committing and therefore does not stay up to
    date with subsequent label changes). Then if we rerun the failed push
    job after amending the labels, the check will pass.

  4. The github.CheckLabels() method should return a boolean (along with
    a message) indicating whether the labels are valid or not for the pull
    request, rather than returning an error. As per:
    https://martinfowler.com/articles/replaceThrowWithNotification.html
    If this method returns false, we'll then return an error in the Mage
    command (which will ensure a non-zero exit code), so that the GitHub
    Action fails the check.

Written in Go, using Mage: https://magefile.org/

Code is loosely based on this example:
https://www.sethvargo.com/writing-github-actions-in-go/

Checks for exactly one of the specified labels, and fails if there are
none or more than one.

Future enhanced functionality could be to allow the client application
to also specify a set of labels none of which should be present, and
a set all of which should be present.

Two noteworthy things to improve in future commits:

1. There is a bug whereby a PR can show that the check has passed, but
still not allow the pull request to be merged.  This is because GitHub
Actions create a separate new instance of the check for some different
event types, e.g. the `labeled` event creates a new check instance
separate from the `push` event checks.  So if we, for instance:
1. push a commit when there is no required label
2. add the required label
then the `lableled` instance of the check will pass, but the push will
still show the initial failure.
The proposed fix for this (in a future commit) is to retrieve the GitHub
labels for the pull request via the GitHub API, instead of using the
labels in the GitHub Action's event JSON (which is a frozen snapshot of
the labels at the time of committing and therefore does not stay up to
date with subsequent label changes).  Then if we rerun the failed `push`
job after amending the labels, the check will pass.

2. The `github.CheckLabels()` method should return a boolean (along with
a message) indicating whether the labels are valid or not for the pull
request, rather than returning an error.  As per:
https://martinfowler.com/articles/replaceThrowWithNotification.html
If this method returns false, we'll then return an error in the Mage
command (which will ensure a non-zero exit code), so that the GitHub
Action fails the check.
@johnboyes johnboyes added the minor Semantic versioning: new functionality that is backwards compatible label May 3, 2020
@johnboyes johnboyes merged commit 093b938 into master May 3, 2020
@johnboyes johnboyes deleted the use-go branch May 3, 2020 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Semantic versioning: new functionality that is backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant