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

Do not run the diff GA workflow on closed PRs. #3437

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

gouttegd
Copy link
Collaborator

The diff.yml GitHub Actions workflow runs whenever a new comment is added to a PR, to check whether the comment contains a gogoeditdiff tag that asks for the production of a diff.

Unfortunately, one of the actions this workflow is dependent on does not handle correctly the case where the PR has been closed but is still commented on, and throws an error when that happens.

So we add an additional condition before running the workflow: the PR has to be open.

This will prevent people from requesting a diff in a closed PR, but:

(1) this should not be something that people would need often (if the PR has been closed, why would a diff be required);

(2) if a diff is required on a closed PR, it is always possible to re-open the PR (arguably this is what should be done anyway: if someone is interested in the PR enough for wanting a diff and checking what are its consequences on the ontology, that sounds like a sign that the PR is not worthless and should be kept open);

(3) in any case, in the current situation requesting a diff on a closed PR already does not work anyway, since the action we are dependent does not know how to work on a closed PR.

The main interest of this change is to avoid receiving needless emails letting us know that a workflow has failed, just because people are commenting on a closed PR.

The `diff.yml` GitHub Actions workflow runs whenever a new comment is
added to a PR, to check whether the comment contains a `gogoeditdiff`
tag that asks for the production of a diff.

Unfortunately, one of the actions this workflow is dependent on does not
handle correctly the case where the PR has been closed but is still
commented on, and throws an error when that happens.

So we add an additional condition before running the workflow: the PR
has to be open.

This will prevent people from requesting a diff in a closed PR, but:

(1) this should not be something that people would need often (if the PR
has been closed, why would a diff be required);

(2) if a diff _is_ required on a closed PR, it is always possible to
re-open the PR (arguably this is what should be done anyway: if someone
is interested in the PR enough for wanting a diff and checking what are
its consequences on the ontology, that sounds like a sign that the PR is
not worthless and should be kept open);

(3) in any case, in the current situation requesting a diff on a closed
PR already does not work anyway, since the action we are dependent does
not know how to work on a closed PR.

The main interest of this change is to avoid receiving needless emails
letting us know that a workflow has failed, just because people are
commenting on a closed PR.
@gouttegd gouttegd self-assigned this Nov 25, 2024
@gouttegd gouttegd requested a review from matentzn November 25, 2024 18:38
Copy link
Contributor

@matentzn matentzn left a comment

Choose a reason for hiding this comment

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

Lol our discussion caused this? Oops. I was wondering what that was about.. Thank you!

@gouttegd
Copy link
Collaborator Author

Lol our discussion caused this? Oops.

Yep! 😄

@gouttegd gouttegd merged commit a77f1aa into master Nov 25, 2024
@gouttegd gouttegd deleted the dont-trigger-gogoeditdiff-on-closed-pr branch November 25, 2024 18:46
@gouttegd
Copy link
Collaborator Author

Hum, there might be something more to it… https://github.com/obophenotype/uberon/actions/runs/12016711939

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.

2 participants