hls4ml
hls4ml copied to clipboard
Create branch on PR
Description
Currently, our GitLab PyTests do not run on PRs created from forks due to the configuration of GitLab CI (i.e. the commit has to exist in our repo). The purpose of this PR is to push the head of a PR branch (especially those made from forks) to a branch in this repo.
This PR adds two GitHub Action workflows
- When a new PR is opened, a branch is created called
pr/#
, where#
is the PR number - When a PR is updated, the head of the PR branch is pushed to the (hopefully existing) branch
pr/#
Type of change
For a new feature or function, please create an issue first to discuss it with us before submitting a pull request.
- [x] Other (Specify): CI update
Tests
Tested on https://github.com/jmduarte/hls4ml
Checklist
- [x] I have read the guidelines for contributing.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have made corresponding changes to the documentation.
- [x] My changes generate no new warnings.
- [x] I have added tests that prove my fix is effective or that my feature works.B
This looks very useful to me. I have a few questions:
- Do you understand why the test is failing?
- What about the transient time, after we merge this, before all the /pr/* branches exist, will this cause errors? If so, is the solution to manually add the /pr/* branches to all currently open branches or to add a check?
- Do we want to limit this to only be done if the PR is from a different fork? It doesn't seem necessary to do it if it's not from a fork, but maybe there are no problems doing it.
- Do we want to delete the branches automatically when we accept or close a PR so that we don't wind up with lots of branches?
I like this, but I think we may still need some manual action involved just as a protection. I've seen examples where a specific comment like "run tests" by an admin in the PR chat triggers the workflow, or there are these.
After some extensive testing I think the current configuration does the right thing:
- If you raise or update a PR from the same repo, the action runs but it does nothing
- If you raise a PR from a fork of the repo, the action runs and it creates a
pr/#
branch - If you update a PR from a fork of the repo, the action runs and it updates the
pr/#
branch
Tests in my fork: https://github.com/jmduarte/hls4ml/pulls
This needs to be merged into main
to take effect on new/open PRs. This is because we're using on: pull_request_target
instead of on: pull_request
, which means the workflow runs from the base repo instead of the head repo to ensure we have permission to push to our own (base) repo.
The one caveat is if a PR from a fork updates the GitHub Actions workflows, then this action will fail because GITHUB_TOKEN
won't have the right permissions to update workflows. I think this is actually sensible for security reasons. If this happens, we can just push the branch ourselves.
@thesps @jmitrevs please review again
We still need some manual component to the action, just a click button or anything to trigger the making of the branch. That’s just to have some oversight to prevent arbitrary code being run on the CI setup.
@thesps I updated it so the action will only be triggered if an admin (or actually, anyone with triage
access to the repo) adds the label please test
to the PR.
Does that work for you?
You can see it in action here: https://github.com/jmduarte/hls4ml/pull/17
Note this won't work in our repo until this is merged.