chdsbd / kodiak

🔮 A bot to automatically update and merge GitHub PRs
https://kodiakhq.com
GNU Affero General Public License v3.0
1.05k stars 63 forks source link

`rebase_fast_forward` performs a standard merge, without rebase #864

Closed 64kramsystem closed 1 year ago

64kramsystem commented 1 year ago

Hello!

I'm experiencing a problem where rebase_fast_forward performs a standard merge, without rebase.

I'm not sure whether this is a misunderstanding of mine or some sort of issue with Kodiak.

My requirements are:

Now, based on my reading of the documents, in order to accomplish this, I need to:

But with the above conditions, I still get a standard merge.

Test case

I've created a main branch with the following history:

* 162bbeb foo
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

then a test branch (PR) with the following history:

* 92dbfec bar
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

after I label the PR with automerge (I tried this with a variable time - immediately, and after a few seconds of creating the PR), I see the button This branch is out-of-date with the base branch for a short time, then Kodiak starts to do its stuff, then for a short time I see a message reporting that there is a conflict with the main branch, then Kodiak merges the PR, with this result:

*   d02fff7 (origin/main, origin/HEAD) Merge branch 'main' into test
|\
| * 162bbeb (HEAD -> main) foo
* | 92dbfec (origin/test, test) bar
|/
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

I was expecting something around the lines of this:

*  cafebabe Merge branch 'main' into test
|\
| * deadbeef bar
|/
* 162bbeb foo
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

In one of my attempts I've seen an octopus merge with three heads, which may be the intended result, but I wasn't able to reproduce it (and I'm not even sure it was correct, or just a mistake).

Am I missing something obvious?

If useful, the PR is this, with some pulls.

chdsbd commented 1 year ago

He @64kramsystem,

Thanks for the report.

The GitHub API only allows Kodiak to update pull request branches using the "Merge" method. So even though you configure rebase_fast_forward for merging your pull request, because you have "Require branches to be up-to-date before merge" enabled, Kodiak will update your branch using the GitHub API before merging.

Would squash merging work for you? This way all of your commits on your PR would be merged together into one commit on your main branch, giving you a linear history on your main branch

64kramsystem commented 1 year ago

Hello, thanks for the reply :smile:

Squash merge doesn't work, as I wanted to retain all the commits in the (local) history. What I'm basically looking for is, before merging, that the bot:

Using an expanded version of the diagrams above, it'd be the following.

Main (simplified; in reality, in the project I work on, pushing on main requires merge commits):

* aaaaaaa foo2
* 162bbeb foo
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

PR branch:

* ccccccc bar2
* bbbbbbb bar
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

Then, on merge, the bot would first rebase (this implies a history rewrite of the PR branch):

* eeeeeee bar2
* ddddddd bar
* aaaaaaa foo2
* 162bbeb foo
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

Then merge:

*  cafebabe Merge branch 'main' into test
|\
| * eeeeeee bar2
| * ddddddd bar
|/
* aaaaaaa foo2
* 162bbeb foo
* 2cfafef Add workflow/kodiak
* 5ceca7c Add geet repo content, for testing (minus spec)

If this is possible, I'm fine with any Kodiak configuration (I'm not bound to other requirements, other than the semi-linear history) :smile:

chdsbd commented 1 year ago

I'm afraid it's not possible to updated a PR via "rebase" using the GitHub API. The API only allows "merge" updates.

Out of curiosity, is there a technical reason you need to keep the commit history from the branch? I've found "squash" merges to work well

64kramsystem commented 1 year ago

I'm afraid it's not possible to updated a PR via "rebase" using the GitHub API. The API only allows "merge" updates.

:cry:

Out of curiosity, is there a technical reason you need to keep the commit history from the branch? I've found "squash" merges to work well

I have a personal habit to be very strict about atomic commits (I estimate 95%+ of my commits are so); as a consequence of this, without squashing, I can very effectively bisect my code when needed, and more generally, look at history in a very granular fashion.

With squash PRs, I still can bisect, but it's a bit of a hassle, because I need to find the culprit PR commit, then find and switch the original branch, then perform a second bisect. Regarding granular history reading, I think that's just off the table with squashed PRs.

All in all though, when compared to merge PRs, semi-linear history is more cosmetic than else, since I (again, personally) rarely need to look at history in a manual/sequential fashion. It looks great, though :joy:

Having said that, atomic commits is a habit that took a long time to exercise/develop. In my team at work nobody does it, so we indeed use squash PRs.

I think atomic commits are very influential on how one thinks programming, so it's a lost opportunity, but well, that's life :laughing:

64kramsystem commented 1 year ago

Closing the issue, since the feature is not supported. Thanks for your support! :pray: