Skip to content

Conversation

@Joe-Downs
Copy link
Member

This adds comments onto the pull request containing what caused the commit checks to fail, if any, and suggests fixes to the user. If no errors are raised, no comment is made.

GitHub says there's a limit of 65536 characters on comments. If the bot's comment is over that limit, it will truncate the comment to fit, and add a message explaining where the remaining errors can be found. Unfortunately, the GitHub API doesn't seem to provide a job's unique ID for linking to a job run (this is different than an action run: ".../runs/..." vs ".../actions/runs/...", respectively), so we can't directly link to the error messages printed to the console. Additionally, to create this link, two new environment variables are used: GITHUB_RUN_ID and GITHUB_SERVER_URL.

Because we need the PR object twice, check_github_pr_description() was also changed to have the PR object passed into it; the PR object is gotten with a new function, get_github_pr().

The GitHub action configuration was changed to run on pull_request_target, instead of pull_request. Here are the differences:

  • The target repo (aka base repo) is cloned instead of the head repo.
    • This means we have to add a git remote for the head repo and fetch it.
  • The action runs in the security context of the target repo,instead of the head repo.
    • This allows us to post a comment on the pull request.
  • This action runs even if there are merge conflicts on the PR.

Similar to 5bf0b02, we restrict permissions of the action for pull_request_target.

Signed-off-by: Joe Downs [email protected]
(cherry picked from commit ef866a4)

This adds comments onto the pull request containing what caused the commit
checks to fail, if any, and suggests fixes to the user. If no errors are raised,
no comment is made.

GitHub says there's a limit of 65536 characters on comments. If the bot's
comment is over that limit, it will truncate the comment to fit, and add a
message explaining where the remaining errors can be found. Unfortunately, the
GitHub API doesn't seem to provide a job's unique ID for linking to a job
run (this is different than an action run: ".../runs/..." vs
".../actions/runs/...", respectively), so we can't directly link to the error
messages printed to the console. Additionally, to create this link, two new
environment variables are used: GITHUB_RUN_ID and GITHUB_SERVER_URL.

Because we need the PR object twice, check_github_pr_description() was also
changed to have the PR object passed into it; the PR object is gotten with a new
function, get_github_pr().

The GitHub action configuration was changed to run on pull_request_target,
instead of pull_request. Here are the differences:

* The target repo (aka base repo) is cloned instead of the head repo.
  * This means we have to add a git remote for the head repo and fetch it.
* The action runs in the security context of the target repo,instead of the
  head repo.
  * This allows us to post a comment on the pull request.
* This action runs even if there are merge conflicts on the PR.

Similar to 5bf0b02, we restrict permissions of the action for
pull_request_target.

Signed-off-by: Joe Downs <[email protected]>
(cherry picked from commit ef866a4)
@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@jsquyres
Copy link
Member

jsquyres commented Sep 11, 2022

See #10786 (comment) for why I am merging this PR even though the required "Git commit checker" CI test is not satisfied, and not even bothering to allow the OMPI AWS Jenkins CI tests to run before merging.

@jsquyres jsquyres merged commit aec94d8 into open-mpi:v4.0.x Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants