search

sorenlouv / backport

A simple CLI tool that automates the process of backporting commits on a GitHub repo
https://github.com/sqren/backport/blob/main/docs/config-file-options.md
Apache License 2.0
247 stars 60 forks source link

backport --all not showing all commits #295

Closed tghosth closed 2 years ago

tghosth commented 2 years ago

Hi @sqren,

I love this tool and it has been really useful for the OWASP ASVS but I am having a problem with it recently.

This is the most recent commit history in the ASVS master branch: image

I have historically run backport using the following command (for example): backport --repo-owner owasp --repo-name asvs --source-branch master --branch v4.0.3 --max-number 100 --multiple true --all

However, when I do this it is not showing me all commits even through I have specified --all: image

It is also not super clear to me how it is choosing which commits to show...

It is possible I was using an older version up beforehand because I changed laptops about a month ago and reinstalled everything so it would be hard to say when exactly this issue started.

Any ideas? Thanks!

tghosth commented 2 years ago

I thought maybe I was on an older version but I checked and I am on backport 6.2.1

sorenlouv commented 2 years ago

Hi @tghosth

Thanks for opening this issue! I looked into this and you are right, it's currently very confusing what commits are displayed and which are not.

The short story is that if a pull request has multiple commits, and it is merged (not squashed) only the last commit from the PR will be displayed. An example is https://github.com/OWASP/ASVS/pull/1176. Only the commit "Remove prefix" shows up in the backport tool.

This is not intentional but I had the assumption that people would squash commits when merging a pull request that they intended to backport

A question for you: do you intend to merge a pull request with multiple commits, and then only backports some of these commits? Or are you going to backport every commit from a particular pull request?

tghosth commented 2 years ago

Hi @sqren, thanks for the quick response :)

So it is likely but not certain that I would be backporting the whole PR and not just some of the commits. However, I guess it is theoretically possible that I would backport only some of the commits if I had a PR which made some changes which are already in the target branch and some changes which are not in the target branch.

Although, I think I tried using the --pr flag and it still didn't pull all commits in the PR.

The main thing that was confusing me was that this appeared to be changed behaviour compared to whatever older version I was using.

sorenlouv commented 2 years ago

The main thing that was confusing me was that this appeared to be changed behaviour compared to whatever older version I was using.

Yeah, that makes sense. I'll try to have a fix up in a bit for you to test out. This should make it possible for you to see all commits, and not just some of them.

Although, I think I tried using the --pr flag and it still didn't pull all commits in the PR.

I looked into this and I'm surprised it doesn't work. Backporting a PR that's been merged using "merge" or "squash" options should work. However, if a PR is merged using "rebase" I don't know of a good way to get all the commits. This is because we currently only backport the merge commit of a PR (code). I'd be open to backporting every commit in a PR if it's been merged using rebase but there's no way to see the merge_method of a PR.

sorenlouv commented 2 years ago

@tghosth Can you try installing the beta version and see if that works better:

npm install backport@6.3.0-beta-2
tghosth commented 2 years ago

@sqren unfortunately the list looks the same as before...

image

tghosth commented 2 years ago

That screenshot is the 2nd time I ran the installer (having first uninstalled the older version) just so it would all show on the same screenshot

sorenlouv commented 2 years ago

@tghosth Can you try running backport --version to make sure you are on the right version? (I noticed you used -G where it probably should be -g when installing)

tghosth commented 2 years ago

🤦‍♂️ sorry, my bad. Looks like the commits are showing correctly now :)

tghosth commented 2 years ago

Thanks so much for your patient help :) @sqren

sorenlouv commented 2 years ago

Awesome to hear! I'll let you play around with the beta version and I'll do some more testing myself so unless we find some problems the next couple of days I'll release it as stable.

sorenlouv commented 2 years ago

Released in 7.0.0. Please don't hesitate to bump this issue, or create a new one if you run into something.