Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow creating multiple backports from one pull request #18

Merged
merged 2 commits into from
Mar 22, 2019

Conversation

m-kuhn
Copy link
Contributor

@m-kuhn m-kuhn commented Mar 3, 2019

Fix #17

Disclaimer: I have never written TypeScript before and have used the github online editor. Needs careful review!

@autorebase autorebase bot removed the autosquash label Mar 22, 2019
Repository owner deleted a comment from autorebase bot Mar 22, 2019
@tibdex tibdex merged commit a24518c into tibdex:master Mar 22, 2019
@m-kuhn m-kuhn deleted the patch-1 branch March 25, 2019 06:21
@m-kuhn
Copy link
Contributor Author

m-kuhn commented Apr 9, 2019

Apparently when backporting to multiple branches, there is only one comment about a backport failure, even if multiple backports fail. This makes it hard to quickly see which backports need to be done manually.

@tibdex do you have an idea why this is? Some throttling on github side maybe?

@tibdex
Copy link
Owner

tibdex commented Apr 9, 2019

It's because I use p-series:

backport/src/backport.ts

Lines 137 to 141 in 35fd5c5

return pSeries(
backportLabels.map(label => () =>
backportForLabel({ label, octokit, owner, pullRequestNumber, repo }),
),
);

And its doc says:

Returns a Promise that is fulfilled when all promises returned from calling the functions in tasks are fulfilled, or rejects if any of the promises reject. The fulfilled value is an Array of the fulfilled values.

It would indeed be better to try them all, even if one rebase failed.

@m-kuhn
Copy link
Contributor Author

m-kuhn commented Apr 9, 2019

That sounds reasonable.
Are you aware of a better alternative? If you are able to give me an initial hint, I can try to make a pull request.

@tibdex
Copy link
Owner

tibdex commented Apr 13, 2019

Tracking this with #27, the fix should be fairly easy.

@m-kuhn
Copy link
Contributor Author

m-kuhn commented Apr 14, 2019

In fact I fail to understand the documentation you cited. From what I can see it mentions the aggregated end result of the promise but doesn't talk about continuation after a rejected function. p-all and friends docs look very similar to me. It's likely related to the fact that the whole concept and terminology with "promises" is new to me.

@tibdex
Copy link
Owner

tibdex commented Apr 14, 2019

You can see the fix in c252bc6, tell me if you don't understand something ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating multiple backports at once
2 participants