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

stack refactoring #3338

Open
3 of 6 tasks
kevgo opened this issue Apr 13, 2024 · 17 comments
Open
3 of 6 tasks

stack refactoring #3338

kevgo opened this issue Apr 13, 2024 · 17 comments
Labels
epic A larger feature that comprises several tickets.

Comments

@kevgo
Copy link
Contributor

kevgo commented Apr 13, 2024

motivation

Not sure if this is something people do a lot or even should should do a lot. But if they do it, they should do it using Git Town. Because Git Town

  • provides a workflow engine to do these complex operations step-by-step, with redoing steps after problems
  • updates associated proposals
  • provides safe undo in case of unexpected issues (which are likely with this type of operation)

how it works

  • all branches to refactor must be in sync
    • the changes made here are complex enough, let's not make them more complex by also syncing
  • all branches to refactor must have simple commits
    • no merge commits
    • people who use the merge sync-strategy need to compress their branch first
  • the --onto option for rebase will do the heavy lifting here
  • update associated proposals

Branch commands

Some of these commands could be top-level commands, but they aren't used often enough to justify being a top-level command.

  • git town set-parent - moves the current branch to a different position in the lineage

    • does what the up, down, first, and last commands do, and more (moving to a different stack)
    • That would be useful because it's debatable what "up" and "down" mean in a stack. Arguments can be made either way depending whether the main branch is at the top or bottom.
    • Using "up" and "down" repeatedly becomes slow because it always has to update the proposals as well.
    UI

    The UI is similar to git town switch, but moves the highlighted branch up and down.

    main
      branch-1
    *   branch-2
          branch-3 
    
  • git town merge - merge two consecutive branches in a stack

    example Given this stack:
    main
     \
      branch-1
       \
        branch-2
         \
          branch-3
    

    And I am on branch-2
    When I run git town merge
    Then I have this stack:

    main
     \
      branch-1
       \
        branch-3
    

    And branch-1 contains the changes of branch-1 and branch-2.

  • git town hoist - detach the current branch from its stack (make it an independent top-level branch)

    example

    Given this stack:

    main
     \
      branch-1
       \
        branch-2
         \
          branch-3
    

    And I am on branch-2
    When I run git town detach
    Then I have this stack:

    main
     \
      branch-1
       \
        branch-3
     \
      branch-2
    
  • git town hack --beam - extract a commit into a new branch

    example Given this stack
    main
     \
      branch-1
    

    And branch-1 contains multiple commits
    And I am on the branch-1 branch
    When I run git town hack --move branch-2
    Then it asks me to select the commits to go into the new branch
    And I have this stack:

    main
     \
      branch-1
     \
      branch-2
    
    UI
    Please select the commit to move into branch-2:
    ( ) commit 1
    (x) commit 2
    ( ) commit 3
    
  • git town append --beam - creates a child branch and beams a commit into it

  • git town prepend --beam - creates a new parent branch and beams a commit into it

    UI
    Please select the commits to move into branch-2:
    [x] commit 1
    [ ] commit 2
    [x] commit 3
    [ ] commit 4
    
@kevgo kevgo added idea stage A ticket at the idea stage, needs more thinking epic A larger feature that comprises several tickets. labels Apr 13, 2024
@kevgo kevgo changed the title branch refactoring stack refactoring Apr 13, 2024
@kevgo
Copy link
Contributor Author

kevgo commented May 4, 2024

We could also use set-parent here: #3473

@kevgo
Copy link
Contributor Author

kevgo commented Nov 12, 2024

Decision needed on the behavior of the git town split command.

Consider this stack:

main
 \
  branch-1

Currently, branch-1 is active, and I run git town split branch-2. The question is: how should the new branch hierarchy look like?

Option A:

main
 \
  branch-1
    \
     branch-2

This structure supports a scenario where I’m working on branch-1 but realize that the last few commits would be better isolated in a separate branch. I then "extract" those commits into branch-2.

Option B:

main
 \
  branch-2
    \
     branch-1

This approach could be useful when splitting branch-1 and inserting a new branch (branch-2) to contain initial changes.

@ruudk you proposed this feature - which of these use cases did you have in mind?

@stephenwade curious about your take on this as well. Thanks!

@ruudk
Copy link
Contributor

ruudk commented Nov 12, 2024

Reading my original issue #3667 and thinking about my day to day, I often start hacking on a feature, and fix things along the way (scouts rule). These fixes are always done in separate commits, and are often independent of the feature. In that case I want to split the branch and move some of the commits to a parent (option B).

Instead of creating a new split command, why not add a --split option to git town prepend and git town append? In that way, you can do option A + B, and decide per use case how you want it. Because prepend/append is basically already what this is, the only difference is to move/split commits.

@kevgo
Copy link
Contributor Author

kevgo commented Nov 12, 2024

That's a pretty brilliant idea, thanks for sharing! 🙏 I love that this allows us to implement this without introducing a new top-level command, as Git Town has a lot of top-level commands already.

We could also add this --split option to git town hack. I (personally) prefer implementing unrelated cleanups in independent top-level feature branches rather than as a parent branch of a larger change. Makes proposing, reviewing, shipping, and/or discarding the change easier.

@ruudk
Copy link
Contributor

ruudk commented Nov 12, 2024

Yeah depending on the use case doing a split on hack makes also sense. So let's add it to prepend append and hack.

@kevgo
Copy link
Contributor Author

kevgo commented Nov 12, 2024

Regarding the naming, we're no longer "splitting" a branch mid-way; instead, we're selectively cherry-picking specific commits into the new branch. Let's find a better name for this workflow. Some ideas:

  • --cherry-pick
  • --move-commits
  • --move

@ruudk
Copy link
Contributor

ruudk commented Nov 12, 2024

My script is called hack-pick. So maybe just --pick?

@stephenwade
Copy link
Member

In my opinion:

For prepend/append, we shouldn't call it --pick/--cherry-pick because it should not cherry-pick, just update refs.

For hack, we shouldn't call it --pick/--cherry-pick because that implies cherry-picking but not removing the commits from another branch.

I suggest --move/--move-commits for hack and --split for prepend/append, since they are really different operations: in hack, you're cherry-picking commits which you don't need to do in prepend or append.

@kevgo
Copy link
Contributor Author

kevgo commented Nov 14, 2024

Another decision we have to make, which might also help with naming the CLI option: we now have two alternative approaches for selecting commits to move to the new branch.

Approach A (the initial concept outlined earlier in this ticket): Split the branch at a specified commit, then move either all commits before or after this split point into the new branch.

Approach B (suggested by @ruudk in #3667): Allow users to select individual commits to move into the new branch using checkboxes, enabling non-sequential commits to be moved.

Approach A is much easier to implement and will cause fewer merge conflicts along the way. Approach B seems more flexible.

When using approach A, it would make sense to name the CLI option --split. When using approach B, --move or --pick makes more sense.

There is also the option of implementing commit-moving as a standalone command, separate from creating new branches. This approach seems the most flexible, as it also allows moving commits between existing branches without creating any new branches. It comes at the cost of having to run two separate Git Town commands when extracting commits into a new branch.

@stephenwade
Copy link
Member

Approach A is much easier to implement and will cause fewer merge conflicts along the way. Approach B seems more flexible.

I think it would be nice to have both approaches. Sometimes I just want to split up what I have but keep them in a stack; sometimes I want to move a few commits out of this stack into an isolated branch.


Also, after thinking about it some more, I think these should both be their own commands.

Splitting a branch: When I realize that a PR is too big, I want to split it up. prepend and append are for adding new branches, so they aren't going to come to mind when I think of what action to take. I would expect a split command.

Having a separate command could also allow for a more flexible workflow, like picking multiple split points to create more than two branches or renaming both branches (e.g. start with branch feature and end with branches feature-1 and feature-2). This is how Graphite's split --by-commit feature worked and I miss it.

Graphite split command documentation: https://graphite.dev/docs/squash-fold-split#split-a-branch-into-multiple-branches

Moving commits: This should be its own command to allow moving commits between existing branches. I've definitely been in the situation of creating a commit for feature A on feature B's existing branch.

It comes at the cost of having to run two separate Git Town commands when extracting commits into a new branch.

It would be nice to have one command to create branch + move commits, because that's probably the most common case for moving commits. Here are some convenience shortcut ideas:

  1. Add a --move flag to prepend/append/hack that means "prepend/append/hack, then move commits".
  2. Add --prepend, --append, --hack flags to move-commits that mean "prepend/append/hack a new branch instead of asking me what branch to move to".
  3. Add "new branch" as an option in the branch selector when running move-commits.

@ruudk
Copy link
Contributor

ruudk commented Nov 22, 2024

@stephenwade I think this makes a lot of sense indeed 👍

@levrik
Copy link

levrik commented Dec 13, 2024

git town split is basically what I need for Git Town to be useful to me. I tend to do many things at once but I usually separate them into commits already before pushing.

Currently I manually turn each commit into a separate branch and PR. spr is a tool which automates exactly this workflow but it also automates the PR creation on GitHub which makes it difficult to later overwrite the PR description with screenshots and other things that can't be part of the commit message beforehand. I also had some other issues with it and dislike not having the individual branches also locally.

Git Town looks much better overall but lacks support for splitting a branch apart later in the process. I know this is open source but still I would like to ask if there are any plans on when this feature will be implemented (a few weeks, months, years away)?

@kevgo
Copy link
Contributor Author

kevgo commented Dec 16, 2024

Thanks, everyone, for your thoughtful input—this discussion has been incredibly insightful! Here’s a quick recap of where we stand based on the feedback so far:

Splitting a branch: not practical; extracting commits: promising

The idea of splitting a branch initially sounded intriguing but seems to address only a narrow set of use cases mentioned in this thread. Given its limited applicability and the additional complexity it would introduce, we’re shelving this concept for now.

Conversely, enabling users to view all commits in a branch and select specific ones (via checkboxes) to move to another branch—either new or existing—addresses all of the discussed scenarios.

Extracting commits into a new branch: highly requested

A key takeaway is the widespread need for extracting specific commits from an existing branch into a new one. The most natural fit for Git Town would be to introduce a --move flag to the existing hack, prepend, and append commands, which already create new branches.

Moving commits to an existing branch: fills a gap

A dedicated git town move command could be useful when someone accidentally commits to the wrong branch. Handling this with vanilla Git commands is notoriously cumbersome, so adding a streamlined solution could fill a gap here.

Let me know if I’ve missed anything or if there’s more we should consider!

@kevgo
Copy link
Contributor Author

kevgo commented Dec 17, 2024

Feedback requested: It would be great if somebody who is already manually moving commits between branches shares how they do it. What’s the sequence of Git commands to move a single commit to another branch and remove it from its original branch?

Context

Consider my workspace contains branches branch-1 and branch-2, and branch-1 contains these commits:

commit-1
commit-2
commit-3

I run git town hack --move branch-2 and select the following commit to move:

[ ] commit-1
[x] commit-2
[ ] commit-3

Desired end state

  • branch-1 should contain these commits:

    commit-1
    commit-3
    
  • branch-2 should contain this commit:

    commit-2
    

possible approaches

ChatGPT suggests the following Git commands:

SHA=<SHA of commit-2>
git checkout branch-2
git cherry-pick $SHA
git checkout branch-1
git rebase --onto "$SHA^" "$SHA"

Open questions

  1. Does this approach work?
  2. Does it cover all edge cases?
  3. Is there a better way to achieve this?
  4. If any descendant branches exist, would they need to be rebased to remove references to commit-2?

@ruudk
Copy link
Contributor

ruudk commented Dec 17, 2024

I use cherry-pick (see #3667 (comment)).

As you can see, if you have the SHA's of the commits, you don't need to checkout to the old branch anymore.

@davidolrik
Copy link

Move down:

  • Append a new branch
  • Cherry-pick commits to new branch
  • Remove commits from source branch
  • Rebase (sync stack)

Move up:

  • Prepend a new branch
  • Cherry-pick commits to new branch
  • Rebase to make commits disappear from source (sync stack)

@stephenwade
Copy link
Member

Feedback requested: It would be great if somebody who is already manually moving commits between branches shares how they do it. What’s the sequence of Git commands to move a single commit to another branch and remove it from its original branch?

I use cherry-pick to move commits, then interactive rebase to remove the commits from the previous branch.

ChatGPT suggests the following Git commands:

SHA=<SHA of commit-2>
git checkout branch-2
git cherry-pick $SHA
git checkout branch-1
git rebase --onto "$SHA^" "$SHA"
  1. Does this approach work?

This works if you are moving only one commit.

  1. Does it cover all edge cases?

It would require multiple rebases if you are moving multiple commits, and I think that might mean multiple resolutions of the same conflict.

  1. Is there a better way to achieve this?

I think a better approach would be to cherry-pick any commits after the first moved one. This would replicate a single interactive rebase, and only require resolving any conflicts once.

  1. If any descendant branches exist, would they need to be rebased to remove references to commit-2?

Yes, any time a rebase happens any descendant branches need to be rebased so they have the new branch tip in their ancestry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic A larger feature that comprises several tickets.
Development

No branches or pull requests

5 participants