Skip to content

[Merged by Bors] - fix(*_comment.yml): fix uses of github.event.issue.pull_request#16241

Closed
bryangingechen wants to merge 2 commits intomasterfrom
bgc-maintainer-merge-fix
Closed

[Merged by Bors] - fix(*_comment.yml): fix uses of github.event.issue.pull_request#16241
bryangingechen wants to merge 2 commits intomasterfrom
bgc-maintainer-merge-fix

Conversation

@bryangingechen
Copy link
Copy Markdown
Contributor

@bryangingechen bryangingechen commented Aug 29, 2024

In our Github actions workflows which react to comments, there were several instances of github.event.issue.pull_request != null to check whether the comment was posted on a PR. However, this always returns true, so these workflows could in principle be triggered to run by someone writing comments on issues rather than PRs.

The Github actions documentation suggests just using github.event.issue.pull_request in conditionals, so I've replaced the incorrect tests with that.

There was also a usage of toJSON(github.event.issue.pull_request) != null which even if correct is overcomplicated, so I fixed that as well.


Open in Gitpod

Follow-up to #15781 (comment)

@bryangingechen bryangingechen added the CI Modifies the continuous integration setup or other automation label Aug 29, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 29, 2024

PR summary bccd4d4ef2

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@bryangingechen bryangingechen changed the title fix(*_comment.yaml): fix incorrect / overcomplicated tests of github.event.issue.pull_request fix(*_comment.yml): fix incorrect / overcomplicated tests of github.event.issue.pull_request Aug 31, 2024
@bryangingechen bryangingechen changed the title fix(*_comment.yml): fix incorrect / overcomplicated tests of github.event.issue.pull_request fix(*_comment.yml): fix uses of github.event.issue.pull_request and line endings Sep 3, 2024
@bryangingechen bryangingechen force-pushed the bgc-maintainer-merge-fix branch from be4ed72 to bccd4d4 Compare September 3, 2024 15:58
@bryangingechen bryangingechen changed the title fix(*_comment.yml): fix uses of github.event.issue.pull_request and line endings fix(*_comment.yml): fix uses of github.event.issue.pull_request Sep 3, 2024
@grunweg
Copy link
Copy Markdown
Contributor

grunweg commented Sep 3, 2024

I checked the documentation: this change makes sense. Thanks for correcting this!
maintainer merge

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 3, 2024

🚀 Pull request has been placed on the maintainer queue by grunweg.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Sep 3, 2024
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Sep 5, 2024

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Sep 5, 2024
mathlib-bors bot pushed a commit that referenced this pull request Sep 5, 2024
In our Github actions workflows which react to comments, there were several instances of `github.event.issue.pull_request != null` to check whether the comment was posted on a PR. However, this always returns `true`, so these workflows could in principle be triggered to run by someone writing comments on issues rather than PRs.

The [Github actions documentation](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only) suggests just using `github.event.issue.pull_request` in conditionals, so I've replaced the incorrect tests with that.

There was also a usage of `toJSON(github.event.issue.pull_request) != null` which even if correct is overcomplicated, so I fixed that as well.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Sep 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(*_comment.yml): fix uses of github.event.issue.pull_request [Merged by Bors] - fix(*_comment.yml): fix uses of github.event.issue.pull_request Sep 5, 2024
@mathlib-bors mathlib-bors bot closed this Sep 5, 2024
@mathlib-bors mathlib-bors bot deleted the bgc-maintainer-merge-fix branch September 5, 2024 04:30
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
In our Github actions workflows which react to comments, there were several instances of `github.event.issue.pull_request != null` to check whether the comment was posted on a PR. However, this always returns `true`, so these workflows could in principle be triggered to run by someone writing comments on issues rather than PRs.

The [Github actions documentation](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only) suggests just using `github.event.issue.pull_request` in conditionals, so I've replaced the incorrect tests with that.

There was also a usage of `toJSON(github.event.issue.pull_request) != null` which even if correct is overcomplicated, so I fixed that as well.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
In our Github actions workflows which react to comments, there were several instances of `github.event.issue.pull_request != null` to check whether the comment was posted on a PR. However, this always returns `true`, so these workflows could in principle be triggered to run by someone writing comments on issues rather than PRs.

The [Github actions documentation](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only) suggests just using `github.event.issue.pull_request` in conditionals, so I've replaced the incorrect tests with that.

There was also a usage of `toJSON(github.event.issue.pull_request) != null` which even if correct is overcomplicated, so I fixed that as well.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
In our Github actions workflows which react to comments, there were several instances of `github.event.issue.pull_request != null` to check whether the comment was posted on a PR. However, this always returns `true`, so these workflows could in principle be triggered to run by someone writing comments on issues rather than PRs.

The [Github actions documentation](https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only) suggests just using `github.event.issue.pull_request` in conditionals, so I've replaced the incorrect tests with that.

There was also a usage of `toJSON(github.event.issue.pull_request) != null` which even if correct is overcomplicated, so I fixed that as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Modifies the continuous integration setup or other automation maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants