spr icon indicating copy to clipboard operation
spr copied to clipboard

Reverse direction of `--count` in `git spr update`, have >1 commits per level in the stacked PR

Open galenhwang opened this issue 3 years ago • 4 comments

First off I have to say I love using spr! Especially with how relatively new this package is.

Some questions:

  1. Can --count in spr update be changed to update from the top of the stack downwards?
  • It's unclear to me why it's updated "bottom of the stack upwards" currently. The reasoning is I'm usually only changing the most recent commits
  • If I were to change --count n parent commits, then that naturally affect all commits coming after it and all subsequent PRs in the stack. Updating the other way around would ensure that the parent commits are untouched when nothing has changed in them. Please let me know if I'm misunderstanding the current use of this command.
  1. Can we have a natural way of having >1 commit per generated PR in the stacked PR? An example workflow would be if I were asked to change A in A -> B -> C. From a reviewer perspective, it would look cleaner if another commit was added to the PR and reflected what the change was to A. This would also avoid the current approach where everything is always force-pushed.
  2. Can we add an option to not git rebase origin/master --autostash on every update? This is cause master can be broken for us occasionally. This also falls in line though with not wanting to pull more changes in from master branch when developing and keeping a feature branch independent.

galenhwang avatar Feb 16 '23 00:02 galenhwang

  1. When you run spr update all the commits in the stack get updated and pull requests created. The idea of --count is in the case that you are currently working on a commit on top of the stack, but you aren't ready to create a pr for it, this way you can update other commits in the stack, update their prs without creating a new pr for the commit that's not ready yet. Personally, I have only used this flag once, it's very rare. As a side note, I do use spr merge --count more often, when I don't want to merge the whole stack, but just a number of commits from the bottom.
  2. No, one commit per pull request is intrinsic in the design of spr.
  3. Yes, this feature has been request by a few people, I'll try to push it into the next release. Side note ,one of the added benefits of having one commit per pull request, is that every commit goes through all the checks and ci. If you adhere to this as a software team, you end up with master that is never broken, as well as a nice linear history without silly fixup commits.

ejoffe avatar Feb 17 '23 06:02 ejoffe

Just want to piggyback on this - when doing spr update is there a way to avoid triggering the CI / github actions on all the commits in the stack?

lazywei avatar Feb 17 '23 21:02 lazywei

For 2, I guess that makes sense since the corresponding command is amend.

How about something where you can auto-add a GH comment for the PR/commit you're amending? If this feature is developed along with the "avoid-rebase-on-update" feature, then naturally reviewers will be able to see the diff between each force-push, and the corresponding GH comment on what was changed, in the PR's timeline.

galenhwang avatar Feb 18 '23 23:02 galenhwang

There is now a noRebase configuration in UserConfig to disable rebase on update. Regarding 2: We can add a --comment argument to git amend which will amend the comment to the commit.

ejoffe avatar Apr 18 '23 18:04 ejoffe