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

Improve the stack branch command to handle trunk branches as parents #344

Closed
wants to merge 1 commit into from

Conversation

draftcode
Copy link
Contributor

@draftcode draftcode commented Jun 17, 2024

When creating a new branch from a trunk branch, the stack branch command
is better to start off from the remote tracking branch instead of the
local branch. This way, the new branch will be created from the latest
state of the trunk branch.

While we are here, accept origin/HEAD as the parent branch name and
resolve it to the default branch.

Previously, this command automatically "adopts" the parent branch to av
automatically, assuming that the parent branch's parent is the default
branch, which might not be true. Instead, this command checks if the
parent branch is already adopted or not.

Copy link
Contributor

aviator-app bot commented Jun 17, 2024

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.

Stack

  1. 👉 Improve the stack branch command to handle trunk branches as parents #344 👈 (this pr)

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.

Copy link
Contributor

aviator-app bot commented Jun 17, 2024

FlexReview Summary

Based on the code complexity and the author's expertise score, these are the suggested reviewers:

  • @jainankit (current review load: 0)

See the list of alternate reviewers in the detailed breakdown below.

Detailed Breakdown Author’s expertise score for the modified files:
  • cmd/av/stack_branch.go (1.00)
  • e2e_tests/stack_orphan_test.go (1.00)
  • e2e_tests/stack_restack_test.go (1.00)
  • e2e_tests/stack_sync_amend_test.go (1.00)
  • e2e_tests/stack_sync_delete_merged_test.go (1.00)
  • e2e_tests/stack_sync_delete_parent_test.go (1.00)
  • e2e_tests/stack_sync_merge_commit_test.go (1.00)
  • e2e_tests/stack_sync_merged_parent_test.go (1.00)
  • e2e_tests/stack_sync_test.go (1.00)
  • e2e_tests/stack_sync_trunk_test.go (1.00)
Files Reviewers
cmd/av/stack_branch.go †,
e2e_tests/stack_orphan_test.go †,
e2e_tests/stack_restack_test.go †,
e2e_tests/stack_sync_amend_test.go †,
e2e_tests/stack_sync_delete_merged_test.go †,
e2e_tests/stack_sync_delete_parent_test.go †,
e2e_tests/stack_sync_merge_commit_test.go †,
e2e_tests/stack_sync_merged_parent_test.go †,
e2e_tests/stack_sync_test.go †,
e2e_tests/stack_sync_trunk_test.go
@jainankit (score: 0.86, current review load: 0)

† Indicates that the file doesn't need an expert review. (?)

See full breakdown of the reviewers on the Aviator webapp.

@aviator-app aviator-app bot requested a review from jainankit June 17, 2024 20:54
@jainankit
Copy link
Contributor

A few questions (user experience related):

  • With this new functionality, would we internally do a git fetch for that remote ref in case our local trunk branch is not up to date?
  • A separate minor UX note that now the behavior is slightly different for trunk branch vs feature branches. We should show this as a message to the user when running av stack branch on trunk.
  • If we report an error when parent is not adopted, we should recommend user with the command to adopt the entire stack. There could be scenario where the user wants to push multiple branches together. Or there could be a param within av stack branch to support that behavior.

@draftcode
Copy link
Contributor Author

A few questions (user experience related):

  • With this new functionality, would we internally do a git fetch for that remote ref in case our local trunk branch is not up to date?

Answered offline, but this is using the remote tracking branches, which is a local ref. It doesn't do any git-fetch.

  • A separate minor UX note that now the behavior is slightly different for trunk branch vs feature branches. We should show this as a message to the user when running av stack branch on trunk.

This is consistent with av-stack-sync behavior.

  • If we report an error when parent is not adopted, we should recommend user with the command to adopt the entire stack. There could be scenario where the user wants to push multiple branches together. Or there could be a param within av stack branch to support that behavior.

I will update the error message.

@draftcode draftcode force-pushed the stack_branch_branch_rtb branch 2 times, most recently from 50ed0b8 to 31edd7d Compare June 25, 2024 20:26
@draftcode draftcode force-pushed the stack_branch_branch_rtb branch 2 times, most recently from 93c82c9 to 175d2a8 Compare June 26, 2024 20:40
When creating a new branch from a trunk branch, the stack branch command
is better to start off from the remote tracking branch instead of the
local branch. This way, the new branch will be created from the latest
state of the trunk branch.

While we are here, accept `origin/HEAD` as the parent branch name and
resolve it to the default branch.

Previously, this command automatically "adopts" the parent branch to av
automatically, assuming that the parent branch's parent is the default
branch, which might not be true. Instead, this command checks if the
parent branch is already adopted or not.
@aviator-app aviator-app bot closed this in ae87eea Jun 26, 2024
@draftcode draftcode deleted the stack_branch_branch_rtb branch August 2, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants