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

Backport commits in PR instead of merge commit #257

Closed
Krzmbrzl opened this issue May 31, 2021 · 7 comments · Fixed by #303
Closed

Backport commits in PR instead of merge commit #257

Krzmbrzl opened this issue May 31, 2021 · 7 comments · Fixed by #303

Comments

@Krzmbrzl
Copy link

Currently when one runs

backport --pr 123

it seems that the tool searches for the merge commit that merged PR 123 into the master branch and then (I assume) it backports that merge commit.

This approach has two drawbacks:

  1. It's only possible to backport PRs that have already been merged
  2. It (presumably) does not allow for an easy way to backport the individual commits of a PR

To 1): We have a few CI runs that take a while to complete and sometimes I already know that I want to backport a PR before these checks have completed (and thus before we have merged the PR). In that case it'd be nice if I could already create the backport PR in order to give its checks a bit of a headstart so I don't have to wait that long for them to complete after having merged the original PR.

To 2): We don't backport merge commits. Instead we cherry-pick the commits of a particular PR on the backport branch and then create a PR from that. It would be nice if there was an easy way to automate this process using the backport tool. (If that is already possible, please let me know how)

@Krzmbrzl
Copy link
Author

I am currently looking into how this could be implemented and to me the most straight forward way to do this would be to let fetchCommitByPullNumer return a Promise<Commit[]> instead of Promise<Commit> and then in there to fetch the commits from the PR instead of fetching the merge commit of that PR.

Given that I have never used TypeScript before (nor a lot of JS) I am having a hard time figuring out how I would go about fetching the commits from a PR. Any chance you could give me some hints? :)

@sorenlouv
Copy link
Owner

Hey @Krzmbrzl

Sorry for not getting back to you before. Is this still something that would be useful for you?

@Krzmbrzl
Copy link
Author

Yes it would indeed still be useful. Atm I'm using backport to backport everything that was introduced in a single-commit PR (for which I can simply refer backport to that commit's hash) but every PR consisting of multiple PRs still have to be backported manually.

@sorenlouv
Copy link
Owner

@Krzmbrzl Have you considered the "squash and merge" option? I'm curious to hear why that isn't suitable for your use case.

@Krzmbrzl
Copy link
Author

I am aware that this option exists but we generally don't use it, because when we have multiple commits in a PR, we actually want to retain those individual commits. If we used squash & merge, we'd have to be more strict about the changes that may be contained within a single PR (we'd only allow one kind of change per PR) whereas now, we have pretty relaxed rules with regards to PRs but instead have those strict rules for the commits (separate commits for separate changes, but only one commit per change).
That is to say that we currently performing the necessary squashing manually which gives us the opportunity to "sneak in" e.g. a commit that fixes an inconsistency in the code that one has found on the way.

So in short: If we used squash & merge in our usecase, we'd screw up the git history as we'd then be squashing (potentially) unrelated changes into a single commit.

@sorenlouv
Copy link
Owner

I've published a beta version of backport that will allow you to backport all the commits in the PR instead of the merge commit:

npm install backport@7.2.0-beta.1

Please try it out and let me know how it goes.

@Krzmbrzl
Copy link
Author

Sorry for not having gotten back to you earlier. Just wanted to let you know that this now seems to work like charm 👍

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 a pull request may close this issue.

2 participants