Add av stack split <new branch> to split last commit into a new branch #472

Chafid · 2024-11-29T09:03:58Z

Adding new feature to split the last commit into a new branch and revert the old branch to 1 previous commit

aviator-app · 2024-11-29T09:04:00Z

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

aviator-app · 2024-11-29T09:04:01Z

✅ FlexReview Status

Owner: aviator-co/engineering (expertise assignment)
Review SLO: 7 business hours if PR size is <= 200 LOC for the first response.

cmd/av/stack_split.go

draftcode · 2024-12-05T22:45:23Z

There's another big change going on. Let me hold this until it's merged.

Chafid · 2024-12-06T07:43:49Z

Noted @draftcode , thanks for the update

cmd/av/stack_split.go

cmd/av/av

cmd/av/stack_split.go

tulioz · 2024-12-07T02:05:02Z

Some more items:

after running av stack split branch-2 we stay on branch-1, we should checkout branch-2 instead so that the user is on the correct head branch (this also resolves staged file oddities in the current setup)
we should make sure we adopt the branch so that we have it stored in av.db
at least for now I believe we're only supporting this action on the tip, so we should ensure there are no child branches so users don't end up in unexpected states

- switch to new branch if split successful

Chafid · 2024-12-07T12:44:09Z

hi @draftcode @tulioz I've updated the PR as per your comments. But I think there's a conflict because of the other branch that got committed in the PR. Should I fix the conflict or you guys should do it?

tulioz · 2024-12-09T23:45:33Z

cmd/av/av

Looks like the binary is still part of the PR, you may have to do some squashing to get rid of it

tulioz · 2024-12-09T23:53:09Z

@Chafid thanks for adding support for no branch name submitted, it looks really good!

To answer your question from this weekend, if you could rebase and migrate the command to av branch --split <optional-name> then this will be almost ready to merge.

Three requests still:

Adding the new branch to the aviator database (essentially av adopt --parent original-branch) so that we are tracking it in av.db
Preventing someone from running this command when not on the tip, so that they don't unexpectedly end up in a state where their stack is branching
Remove the binary file from the PR

Chafid · 2024-12-12T14:26:34Z

Hi @tulioz , I migrated the split command to branch, and I added av adopt command once the split is finished.
I also add additional check to make sure that the user is on the latest commit. If not the command will fail

tulioz

Thanks for joining everything under the av branch command, and addressing the other feedback. Left one comment for code change, and then this following scenario still seems to not work as I'd expect.

av branch commit-1

echo "First commit" > commit-1.txt
git add commit-1.txt
git commit -m "commit 1"

echo "Second commit" > commit-2.txt
git add commit-2.txt
git commit -m "commit 2"
av branch --split

av switch commit-1

echo "Third commit" > commit-3.txt
git add commit-3.txt
git commit -m "commit 3"
av branch --split

av tree

Looks like we still get into a split state when we'd expect that on trying to split commit 3 we'd get an error since the branch commit-1 is not the tip any more

tulioz · 2024-12-12T21:34:20Z

cmd/av/branch.go

+	)
+
+	// Adopt new branch to av database
+	err = adoptCmd.RunE(nil, []string{"parent", currentBranchName})


We can call adoptForceAdoption(repo, db, newBranch, oldBranch) directly

Hi @tulioz , I update to use adoptForceAdoption for updating the av database.
Also, maybe I misunderstand what you mean by "tip of the branch". I assumed that the branch needs to be on its last commit for this split command to run. Is this correct? Because if that's correct then this should work as expected.
When you switch to another branch then the user automatically is on the last commit of that branch, so the split should work.
But if you switch to another commit, like this:

chafid@WINDOWS-4D6659B:~/repository/test_repo$ av switch commit-2 chafid@WINDOWS-4D6659B:~/repository/test_repo$ git log --oneline ccf90a5 (HEAD -> commit-2) commit 2 6714a51 (commit-1) commit 1 0e32223 (origin/main, origin/HEAD, main) 1st commit chafid@WINDOWS-4D6659B:~/repository/test_repo$ av switch 6714a51 chafid@WINDOWS-4D6659B:~/repository/test_repo$ echo "Third commit" > commit-3.txt chafid@WINDOWS-4D6659B:~/repository/test_repo$ git add commit-3.txt chafid@WINDOWS-4D6659B:~/repository/test_repo$ git commit -m "commit 3" [detached HEAD e17cd79] commit 3 1 file changed, 1 insertion(+) create mode 100644 commit-3.txt chafid@WINDOWS-4D6659B:~/repository/test_repo$ av branch --split The split operation was aborted. The original branch remains intact.

This should work as expected

add condition to check detached head

tulioz · 2024-12-19T22:45:09Z

@Chafid sorry for the delay in my response, been a hectic week for me. You are correct we want to force tip of branch, but I think we should also enforce tip of stack. Today if a stack looks like this:

master -> first_branch -> second_branch

and someone runs the command on first branch they'll end up with

master -> first_branch -> second_branch
                      | -> third_branch

and now they have a fork to resolve, ideally we'd only want to let them run on the tip of second_branch so that their stack can only end up like this

master-> first branch -> second_branch -> third_branch

Let me know what you think!

Chafid · 2024-12-20T04:00:19Z

Ah got it @tulioz . Is there already a mechanism in av to check whether the user is already on the tip stack? Or I need to figure it out myself?

Chafid · 2024-12-20T14:16:11Z

Hi @tulioz , I've added the check to only split when in tip of the stack. Please check it

tulioz · 2024-12-20T22:10:12Z

Hey @Chafid, there should be code in av next that does checking for being on tip of stack. I ran the same test as earlier:

av branch commit-1

echo "First commit" > commit-1.txt
git add commit-1.txt
git commit -m "commit 1"

echo "Second commit" > commit-2.txt
git add commit-2.txt
git commit -m "commit 2"
av branch --split

av switch commit-1

echo "Third commit" > commit-3.txt
git add commit-3.txt
git commit -m "commit 3"
av branch --split

av tree

and I'm still seeing the forking behavior:

Chafid · 2024-12-21T06:12:52Z

Hi @tulioz , are you sure you have the latest update? It works correctly on my local

chafid@WINDOWS-4D6659B:~/repository/test_repo$ av tree

  * test3
  │ No pull request
  │
  * test-2
  │ No pull request
  │
  * main (HEAD)

chafid@WINDOWS-4D6659B:~/repository/test_repo$ av switch test-2
chafid@WINDOWS-4D6659B:~/repository/test_repo$ av tree

  * test3
  │ No pull request
  │
  * test-2 (HEAD)
  │ No pull request
  │
  * main

chafid@WINDOWS-4D6659B:~/repository/test_repo$ av branch --split

The split operation was aborted.
branch 'test-2' has a descendant branch 'test3'

error: split failed: branch 'test-2' has a descendant branch 'test3'
chafid@WINDOWS-4D6659B:~/repository/test_repo$

tulioz · 2024-12-23T22:55:00Z

I just double checked that I'm on latest, and it looks like I am

Just double checked and on my local test env I'm still getting the forking behavior 😞

❯ av branch --debug --split
DEBU[0000] enabled debug logging                         av_version="<dev>"
DEBU[0000] git [rev-parse --git-common-dir]              duration=10.034333ms repo=og-testing
DEBU[0000] loaded Git repo                               git_common_dir=/Users/ofergoldstein/dev/og-testing/.git
DEBU[0000] loaded config file                            config_file=/Users/ofergoldstein/dev/og-testing/.git/av/config.yaml
DEBU[0000] git [status --porcelain=v2 --branch --untracked-files]  duration=12.645417ms repo=og-testing
DEBU[0000] git [merge-base 7fbd11068742b5393d6cb0df88796e528b3bf994 cc70206ded0423ac274ef928232252b54afb687b]  duration=10.553709ms repo=og-testing
DEBU[0000] git [merge-base 7fbd11068742b5393d6cb0df88796e528b3bf994 5401d3e3e11e91a41e70723f37b484a305f5578e]  duration=9.040042ms repo=og-testing
Successfully split the last commit into a new branch commit-3.
DEBU[0000] git [symbolic-ref refs/remotes/origin/HEAD]   duration=8.332584ms repo=og-testing
DEBU[0000] git [symbolic-ref refs/remotes/origin/HEAD]   duration=7.356583ms repo=og-testing
DEBU[0000] git [merge-base commit-1 commit-3]            duration=7.308417ms repo=og-testing
DEBU[0000] command exited                                duration=89.414458ms
DEBU[0000] Skipping CLI version check (development version)

Here are my debug logs for the final split that generates the fork.

Chafid · 2024-12-24T15:06:46Z

Hi @tulioz , sorry I just managed to fix this. I finally use av next to check and it should work ok on your scenario now. Please check it. Thanks

tulioz

Thanks for making all of those changes, looks good!

add split last commit to a new branch feature.

d626fbe

Chafid requested a review from a team as a code owner November 29, 2024 09:03

aviator-app bot requested a review from tulioz November 29, 2024 09:04

tulioz requested a review from draftcode December 2, 2024 18:42

update with using go-git

b63db8a

draftcode approved these changes Dec 4, 2024

View reviewed changes

cmd/av/stack_split.go Outdated Show resolved Hide resolved

remove list of branches as per comments

5a80b21

draftcode approved these changes Dec 5, 2024

View reviewed changes

tulioz reviewed Dec 6, 2024

View reviewed changes

cmd/av/stack_split.go Outdated Show resolved Hide resolved

tulioz requested changes Dec 6, 2024

View reviewed changes

cmd/av/av Outdated Show resolved Hide resolved

cmd/av/stack_split.go Outdated Show resolved Hide resolved

tulioz added a commit that referenced this pull request Dec 7, 2024

integrate #472 into branch command with '--split'

565973d

- enable generate branch name if not provided

cf7e29a

- switch to new branch if split successful

tulioz reviewed Dec 9, 2024

View reviewed changes

cmd/av/av Outdated

Copy link

Contributor

tulioz Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the binary is still part of the PR, you may have to do some squashing to get rid of it

Chafid added 4 commits December 12, 2024 12:45

remove av binary

1282756

Merge branch 'master' into issue-397

2ba5452

migrate split to branch

c0c3889

add check if user is not on latest commit

c004af6

tulioz reviewed Dec 12, 2024

View reviewed changes

use adoptForceAdoption

a1474fe

add condition to check detached head

add checking tip stack

d0e12ef

fix issue with tip branch check

e1e1e98

draftcode approved these changes Jan 2, 2025

View reviewed changes

tulioz approved these changes Jan 2, 2025

View reviewed changes

Merge branch 'master' into issue-397

ba1c52f

aviator-app bot merged commit 97e844a into aviator-co:master Jan 2, 2025
3 checks passed

jainankit mentioned this pull request Jan 3, 2025

Add a way to split the last commit as a separate branch #397

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add av stack split <new branch> to split last commit into a new branch #472

Add av stack split <new branch> to split last commit into a new branch #472

Chafid commented Nov 29, 2024

aviator-app bot commented Nov 29, 2024 •

edited

Loading

aviator-app bot commented Nov 29, 2024 •

edited

Loading

draftcode commented Dec 5, 2024

Chafid commented Dec 6, 2024

tulioz commented Dec 7, 2024

Chafid commented Dec 7, 2024

tulioz Dec 9, 2024

tulioz commented Dec 9, 2024 •

edited

Loading

Chafid commented Dec 12, 2024

tulioz left a comment

tulioz Dec 12, 2024

Chafid Dec 14, 2024 •

edited by tulioz

Loading

tulioz commented Dec 19, 2024

Chafid commented Dec 20, 2024

Chafid commented Dec 20, 2024

tulioz commented Dec 20, 2024

Chafid commented Dec 21, 2024 •

edited

Loading

tulioz commented Dec 23, 2024

Chafid commented Dec 24, 2024

tulioz left a comment

Add av stack split <new branch> to split last commit into a new branch #472

Add av stack split <new branch> to split last commit into a new branch #472

Conversation

Chafid commented Nov 29, 2024

aviator-app bot commented Nov 29, 2024 • edited Loading

Current Aviator status

aviator-app bot commented Nov 29, 2024 • edited Loading

✅ FlexReview Status

draftcode commented Dec 5, 2024

Chafid commented Dec 6, 2024

tulioz commented Dec 7, 2024

Chafid commented Dec 7, 2024

tulioz Dec 9, 2024

Choose a reason for hiding this comment

tulioz commented Dec 9, 2024 • edited Loading

Chafid commented Dec 12, 2024

tulioz left a comment

Choose a reason for hiding this comment

tulioz Dec 12, 2024

Choose a reason for hiding this comment

Chafid Dec 14, 2024 • edited by tulioz Loading

Choose a reason for hiding this comment

tulioz commented Dec 19, 2024

Chafid commented Dec 20, 2024

Chafid commented Dec 20, 2024

tulioz commented Dec 20, 2024

Chafid commented Dec 21, 2024 • edited Loading

tulioz commented Dec 23, 2024

Chafid commented Dec 24, 2024

tulioz left a comment

Choose a reason for hiding this comment

aviator-app bot commented Nov 29, 2024 •

edited

Loading

aviator-app bot commented Nov 29, 2024 •

edited

Loading

tulioz commented Dec 9, 2024 •

edited

Loading

Chafid Dec 14, 2024 •

edited by tulioz

Loading

Chafid commented Dec 21, 2024 •

edited

Loading