spacedentist / spr

Submit pull requests for individual, amendable, rebaseable commits to GitHub
https://getcord.github.io/spr/
MIT License
377 stars 33 forks source link

Support using `spr` from a fork #110

Closed joneshf closed 1 year ago

joneshf commented 1 year ago

We make it possible for outside collaborators to use spr.

The vast majority of the changes here are in adding some tests around the changes in src/config.rs. Aside from that, this is a moderately-sized change "lines of code"-wise. The main thing here is that we're taking in optional values for all of the upstream owner, upstream repo, upstream remote, and upstream trunk. Even though they're all optional, it's unclear what it would mean to only have one of the four and use it. E.g. If we only have upstream owner, and we didn't check the others existed, we might do something like push a branch to origin and try to create a PR against an upstream that didn't exist. Also, even though the origin and upstream trunks should likely be the same, we still take it in just to be certain.

To make most things easier, the owner, repo, and trunk don't need to quantify whether it's for origin or upstream outside of Config. All the external cases where those values are used, we want to use the upstream if it exists and fallback to the origin otherwise. Internally, we do make a distinction just because there are things we do specifically on the origin no matter what.

There are two things we seem to have to make an active decision on: the git remote name, and the pull request head.

For the remote name, it seems we always want to push to origin. And, it seems like we always want to use upstream for landing/updating if it exists and fallback to origin otherwise.

For the pull request head, it seems we need to construct some special syntax if there's an upstream repo. It needs to mention the origin repo, so it ends up looking like <origin>:<branch> instead of just <branch>.

All of these changes together mean that we should be able to use spr from a fork. Importantly, it means that spr should now be able to be used by outside collaborators to contribute to spr itself. The only part that's maybe a bit unclear is whether the stacked workflow will actually work.

The API documentation seems to mention that the base parameter has to exist in the current repository: https://docs.github.com/en/rest/pulls/pulls#create-a-pull-request--parameters. Given the way stacking works with spr, it's unclear whether this will work or not. We're optimistically assuming it will work. But we might have to make some changes after the fact to support this flow.

Finally, we update the documentation to mention that there's a way to use spr from a fork and mention the configuration values in the configuration reference documentation.

Test Plan: There's two ways to view this change as working: recognize that I'm making this change from a fork, and the fact that I can submit it is the proof that it's working; or follow these steps yourself:

  1. Create a fork of a repo that you do not have access to. You can use: https://github.com/joneshf/spr-issue-80 if you need a repo.
  2. Setup the configuration as mentioned in the new docs/user/fork.md file.
  3. Follow the simple workflow in docs/user/simple.md to make a simple change and submit it to the upstream repo.
joneshf commented 1 year ago

Whoops. Wasn't supposed to make a new PR. Ignore this.