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

Github Actions, use pull_request and workflow_run instead of pull_request_target #1279

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

Scottmitch
Copy link
Member

Motivation:
pull_request_target will run against the commit where a PR is branched
off of. This may result in errors not being detected until after the
change is merged (e.g. checkstyle, etc.).

Modifications:

  • Instead use the pull_request event to build pull requests, and publish
    the artifacts which need post analysis (test results, checkstyle, etc.).
    A workflow_run job can run with the permissions of the original repo and
    post annotations to PRs indicating any issues with the build. This
    separation is done for security reasons to prevent forks from getting
    access to credentials/secrets.

Result:
Pull requests build/analysis is done on the code in the pull request,
instead of from the target branch.

…uest_target

Motivation:
pull_request_target will run against the commit where a PR is branched
off of. This may result in errors not being detected until after the
change is merged (e.g. checkstyle, etc.).

Modifications:
- Instead use the pull_request event to build pull requests, and publish
the artifacts which need post analysis (test results, checkstyle, etc.).
A workflow_run job can run with the permissions of the original repo and
post annotations to PRs indicating any issues with the build. This
separation is done for security reasons to prevent forks from getting
access to credentials/secrets.

Result:
Pull requests build/analysis is done on the code in the pull request,
instead of from the target branch.
name: Checkstyle Report JDK ${{ matrix.java }}
path: '**/build/reports/checkstyle/*.xml'
token: ${{ secrets.GITHUB_TOKEN }}
# TODO https://github.com/jwgmeligmeyling/checkstyle-github-action/issues/6
Copy link
Member Author

@Scottmitch Scottmitch Dec 14, 2020

Choose a reason for hiding this comment

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

note we will not get the quality reports or GitHub annotations until the Github Actions resolve these issues.

@Scottmitch
Copy link
Member Author

tested locally in my fork, I will merge this since our current approach is broken and doesn't detect checkstyle and other issues on the PR.

@Scottmitch Scottmitch merged commit 50da212 into apple:main Dec 14, 2020
@Scottmitch Scottmitch deleted the gh_actions_workflow_run branch December 14, 2020 21:05
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.

1 participant