ejoffe / spr

Stacked Pull Requests on GitHub
MIT License
796 stars 68 forks source link

Why does adding a new commit affect PRs on commits earlier in the stack? #281

Open WesleyYue opened 1 year ago

WesleyYue commented 1 year ago
  1. I have commits a - b - c, each with their own PR opened via spr update
  2. I add a new commit a - b - c - d. The a, b, c branches are all updated when I run spr update, triggering CI even though nothing has changed in those branches.

Am I setting up something incorrectly?

thomasdesr commented 1 year ago

I also have the same issue, from manually bisecting versions I think it looks like this broke between 0.8.5->0.9.0.

thomasdesr commented 1 year ago

Okay bisected with:

# rebuild loop
rm bin/*
for p in ./cmd/*; do
    echo $p
    go build -o bin/$(basename $p) $p
done
mv bin/commithook bin/spr_commit_hook
mv bin/reword bin/spr_reword_helper

&&

#!/usr/bin/env bash

set -euo pipefail

set -x

BIN_DIR="$PWD"

export PATH="$BIN_DIR/bin:$PATH"

# Head over to test repo
cd "../spr-tests"

ID="$RANDOM"

rm -f .git/hooks/commit-msg

git checkout -b "$ID" main
for i in $(seq 1 4); do
    touch "${ID}-${i}"
    git add "${ID}-${i}"
    git commit -m "${ID}-${i}"
done

spr update

echo "Hi" >> "${ID}-2"
git add -u "${ID}-2"
git absorb
git rebase -i main

sleep 5

spr update

Bisect is pointing to: https://github.com/ejoffe/spr/commit/e7d2462d3e3d67bbe816b0921bd02f8d562b8356

The commit message might be a good pointer?

Remove need for commit-msg hook
instead use rebase with reword helper to add missing commit-ids
thomasdesr commented 1 year ago

Okay I actually see this as fixed as of https://github.com/ejoffe/spr/commit/6f93850d1f6436aaa8b9c8fd3455d9d4847503e4 so if you're still around @WesleyYue try rebuilding from master and give it another go. Its looking fixed on my end if I run off HEAD.

WesleyYue commented 1 year ago

Thanks for digging into this! Will check it out.

thomasdesr commented 1 year ago

That said HEAD isn't in a release xD

So I might still leave this open so its easier for folks to find.

ejoffe commented 1 year ago

Hi Guys, Sorry I haven't had much time recently as have I been really busy with my startup. I have tagged a new release v0.9.3 with the latest master, let me know if this solves your issues.

thomasdesr commented 1 year ago

Thanks for the release, it looks good here!

And no worries about being a bit absent; many thanks for putting together the only stacked PR tool I've found that doesn't push you away from normal git workflows ❤️

WesleyYue commented 1 year ago

I think this issue regressed sometime between 0.9.3 and 0.12.2.