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

git spr update --count missed some commits at the base and created PRs for items higher up the stack instead. #425

Open
chriscz opened this issue Sep 3, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@chriscz
Copy link
Collaborator

chriscz commented Sep 3, 2024

I recently used update --count X and it skipped creating PRs from the base of my branch and then continued to create PRs for things I didn't want, i.e. items beyond the number that I specified, X+1, X+2 etc.

I will try to get a reproduction before I merge my work, and submit a fix.

Possibly related tickets:

@chriscz chriscz self-assigned this Sep 3, 2024
@chriscz chriscz changed the title git spr update --count missed some commits ad the base and created PRs for items higher up the stack instead. git spr update --count missed some commits at the base and created PRs for items higher up the stack instead. Sep 3, 2024
@chriscz
Copy link
Collaborator Author

chriscz commented Sep 3, 2024

It looks like alignLocalCommits in spr/spr.go is dropping these commits because they are not aligning with what is already in the PR on GitHub.

This change was introduced to support merge queues: 5f9733c

@chriscz chriscz added the bug Something isn't working label Sep 3, 2024
@chriscz
Copy link
Collaborator Author

chriscz commented Sep 4, 2024

I tried for a while to debug this though my go is quite rusty and didn't manage to fix it yet.

  • spr was trying to update the stack incorrectly and crashed. Consequently I saw some of the PR targets changed to develop and no longer seem to be stacked
  • After this initial crash spr just tries to create new PRs for everything, probably creating a completely new stack
  • I also notice that all of the prior PRs are no longer being returned in the method below. Instead only a single one is returned.
    func matchPullRequestStack(

spr probably needs some robustness / recovery in a scenario like this. Possibly looking to the source branch (spr/main/xxxx) for guidance on what the real commit id is, it may already be doing this and I missed it.


My PR stack is currently a mess with targets having been incorrectly set and I'm not unable to get it fixed. I'm going to put a hold on using spr for the moment and maybe look at this again in the future. It's a bit unfortunate as I was enjoying it so much.

I'll see if the last thing I can do is create a decent reproduction, but that may only be in a month or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant