Open pull-requests when testing

[dominykas]

Ref: https://github.com/pkgjs/wiby/blob/master/docs/DRAFT-FLOW.md step 9

Open a draft PR in the dependents repository that contains the patched package.json.

Depending on the CI setup, it might not be enough to just push the branch - wiby.json should allow setting a configuration option to additionally open a PR.

This also means that we'd probably need to close the PR, etc, when the tests are done.

[dominykas]

@ghinks and maybe this one?

[ghinks]

ok, will look at this one over the next few days.

[dominykas]

Let me know if you want to discuss it beforehand!

[ghinks]

I think we should discuss it. Let me re-read and respond today. Been super busy.

[ghinks]

Had a quick look, implementation looks straightforwards enough. However there will always be questions.

Options

• CLI Option Name. Should we have a command line option like pull-request?
• Wiby config option. Should we have an option in the configuration pull-request: boolean?

There are some gotchas.

• Just because we have a github token it does not mean it can create a PR. Do we want to check , not sure we can yet but will look into it
• Should we check that the PR has not already been raised? I think when I played with the API it would not let me raise the same PR twice even with different titles

I'll put something together in draft.

[ghinks]

@dominykas @rodion-arr I was wondering if it would be a good idea to split this into 2 parts.

• the first raising the draft PR
• the second checking and closing the PR if the tests passed

your thoughts?

I'm trying to avoid bigger PRs.

[rodion-arr]

It looks like pretty much separate features, imo. I'm not against separate PRs but maybe it's better to release them together (if we have any users at all at this stage)

[ghinks]

yes agreed, release together. But I shall raise 2 PRs.

[dominykas]

CLI Option Name. Should we have a command line option like pull-request?

Yeah, CLI convention is for words to be separated by - in the args, and Yargs will automatically camelCase that when using in the code.

Wiby config option. Should we have an option in the configuration pull-request: boolean?

I suppose since Yargs transforms that into camelCase, the wiby.json should also use pullRequest?

Just because we have a github token it does not mean it can create a PR. Do we want to check , not sure we can yet but will look into it

I'm not sure there's a need to check this, but the GH token will have to have push access (for now) to be able to create the branch - and I'm pretty positive if you have push access, you have PR access? Even in the future, we'll need to have push access into a fork even if we want to PR into upstream, so we'll fail way before this if the token doesn't have correct permissions.

Should we check that the PR has not already been raised? I think when I played with the API it would not let me raise the same PR twice even with different titles

That's a good question.

At the moment, it's not a problem, because we still don't have support for re-using existing branches (bullet point in #78), so we'd fail before the check anyways.

Going forward, though, I think we'll need that check, yes - you can only have a single PR from the same target to the same base (i.e. our test branch to default branch), so it would probably fail if the PR already exists.

the first raising the draft PR
the second checking and closing the PR if the tests passed

Yeah, I think it's fine to do it as separate PRs, whichever you feel like.

Just to clarify - there's no need to do any specific status checks (if that's what you meant), because status checks belong on commits.

We will need to update wiby clean to close the PRs (if any) before deleting the branches. I wonder if getExistingRepoBranches can return the PRs associated with a branch 🤔

[dominykas]

Added in #93 - will release once #100 is merged and master is green again.