You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
GitHub Actions: Use new pull_request_target event #765
In order to solve this [security issues with pull_request], we’ve added a new pull_request_target event, which behaves in an almost identical way to the pull_request event with the same set of filters and payload. However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request. This means the workflow is running from a trusted source and is given access to a read/write token as well as secrets enabling the maintainer to safely comment on or label a pull request.
Switching this in to the .github/workflows/branch.yml workflow should mean that we can have automated comments for PRs coming from forks, rather than just branches 🎉
I would like to also use it in linting.yml, which would that we could have the nice automated comment with the nf-core lint results from fork PRs. However, I think that we want lint tests to run on the merge commit code, so we probably shouldn't switch this one 😞
The new workflow_run event enables you to trigger a new workflow when one or more workflows are requested or completed. Runs triggered by the workflow_run event always use the default branch for the repository, and have access to a read/write token as well as secrets. As an example, as a maintainer you could set up a workflow that takes the artifacts generated by the pull request workflow, do some analysis, and post comments back to the pull request.
There must be a blog post or example somewhere that we can pinch to try this out..
log.warning("Could not post GitHub comment: '{}'\n{}".format(r.status_code, response_pp))
exceptExceptionase:
log.warning("Could not post GitHub comment: {}\n{}".format(os.environ["GITHUB_COMMENTS_URL"], e))
Specifically, it fetches the PR comments to see if there is a linting comment already. If there is, it updates it. If there isn't, it posts a new one. We'd need to figure out how to do this using a GitHub Actions workflow separate from the nf-core lint command if we take this new approach.
Editing instead of reposting is important as otherwise there will be a comment for every commit which is super spammy.
Ok, I can't resist a good challenge, so I got it to work. Took a lot of trial and error (thank you new nf-core/testpipeline!) but seems to be working now 🚀
Activity
ewels commentedon Nov 6, 2020
NB: Tricky to test, as needs to be merged to
masterin order to be active.Use pull_request_target event for branch protection CI
ewels commentedon Nov 6, 2020
This blog post describes how we could get the lint comment to work: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
There must be a blog post or example somewhere that we can pinch to try this out..
ewels commentedon Nov 10, 2020
Tested the linting with the new event trigger and it didn't work, as expected: nf-core/testpipeline#7
So now we know..
ewels commentedon Nov 10, 2020
I started looking into the
workflow_runthing but it's actually fairly complex. The code I wrote to post the comments is not trivial:tools/nf_core/lint.py
Lines 1473 to 1525 in 9b3eec9
Specifically, it fetches the PR comments to see if there is a linting comment already. If there is, it updates it. If there isn't, it posts a new one. We'd need to figure out how to do this using a GitHub Actions workflow separate from the
nf-core lintcommand if we take this new approach.Editing instead of reposting is important as otherwise there will be a comment for every commit which is super spammy.
ewels commentedon Nov 11, 2020
Ok, I can't resist a good challenge, so I got it to work. Took a lot of trial and error (thank you new
nf-core/testpipeline!) but seems to be working now 🚀ci: Try to fix deployment from forks