feat: add ability to open PRs from forked repo - #216
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success220 tests passing in 18 suites. Report generated by 🧪jest coverage report action from b2f17a8 |
| if (backportPR.pushRemoteUrl) { | ||
| await git.gitCli.push(configs.folder, backportPR.head, backportPR.pushRemoteUrl); | ||
| } else { | ||
| await git.gitCli.push(configs.folder, backportPR.head); | ||
| } |
There was a problem hiding this comment.
No need to have the if-else here, if the pushRemoteUrl is undefined the gitCli.push should fallback to origin by default
There was a problem hiding this comment.
Without this construct, there was A LOT of noise in the tests, since they check for this function being called with two parameters in a lot of places.
So I decided for this method to keep the patch footprint low.
I can also update all the tests if that's preferred.
|
|
||
| const sanitized = bpRepo.trim(); | ||
| const parts = sanitized.split("/").map(p => p.trim()).filter(p => p.length > 0); | ||
| if (parts.length !== 2) { |
There was a problem hiding this comment.
This might be a limitation on Gitlab side where we can have nested paths. We can leave it as it is for now and improve it in some follow-up once we can validate on Gitlab side. Commenting just for trackign purposes.
| options.push("--force-with-lease"); | ||
| } | ||
| await this.git(cwd).push(remote, branch, options); | ||
| await this.git(cwd).push(this.remoteWithAuth(remote), branch, options); |
There was a problem hiding this comment.
Instead of running push with token auth which is not needed in the case of origin, as the token is already embedded in the remote - have you considered following a similar approach sued for origin? i.e., call addRemote to add the fork remote using the auth token so that here we don't need this explicit remoteWithAuth call.
There was a problem hiding this comment.
Both approaches worked fine when I tested it.
The forked repos remote was just only used in a single place, and push happily pushes to a direct URL.
And since named remote names won't ever contain ://, this seemed a practical solution to me.
I can also change it to first add a named remote, and then push to that if preferred.
|
I will merge it and then we can follow up with some fixes / improvements if needed |
2 participants
Fixes #27
Description
This adds the ability to have the backport PRs be opened from a separate repository, so the main repository stays clean of backport branches.
This will not work with the automatic actions token. So a manually created PAT for some Bot-Account needs to be provided via secrets, and a forked repo under its control has to exist.
How Has This Been Tested?
Notably, the gitlab part is untested.
Otherwise, this has been deployed to our Forgejo instance at code.ffmpeg.org and has successfully opened backport PRs like https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23855
Checklist
Merge criteria: