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

fix: Do not use set-upstream #88

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Conversation

draftcode
Copy link
Contributor

Each local git branch can have an optional upstream branch. While the
name looks like the push destination of the branch, it's not. Unless
user specifically configure so, git push pushes a branch to the same
name branch on the remote (the simple strategy as described in man 1 git-push). This explains the behavior in the TODO comment in
internal/actions/pr.go "I think currently things will break if the
upstream name is not the same name as the local", because git's default
strategy is pushing to the same branch, not to the upstream branch.

The upstream branch is rather used when doing git rebase. Without an
argument, it rebases to the upstream branch. Likewise, when git pull
does the rebase, it'll rebase to the upstream branch. In many cases, if
a PR is rebased, the intention would be to rebase to the trunk branch
(master/main), not the same branch on the remote.

Based on this, it's not necessary for av-cli to modify the branch
upstream. It's not related to git-push (in most of the cases). Leaving
it intact should allow users to choose whichever branch that makes sense
to them (I personally use origin/main because it makes git rebase
easy).

This should not affect most of the users. Most of them won't notice
the change. The only case they are affected is (1) They do not use av pr create but use av stack sync by some means creating a branch
metadata and (2) set the upstream branch to a different name manually
and (3) pushing to a different name branch manually or setting
"upstream" strategy explicitly in the git config. Again, it's very
unlikely this happens because av pr create did follow the "simple"
strategy rule, which is git's default, and in order for a user to be
affected by this change, they somehow need to get around it to use the
"upstream" strategy.

@aviator-app
Copy link
Contributor

aviator-app bot commented Apr 20, 2023

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.

Each local git branch can have an optional upstream branch. While the
name looks like the push destination of the branch, it's not. Unless
user specifically configure so, `git push` pushes a branch to the same
name branch on the remote (the simple strategy as described in `man 1
git-push`). This explains the behavior in the TODO comment in
internal/actions/pr.go "I think currently things will break if the
upstream name is not the same name as the local", because git's default
strategy is pushing to the same branch, not to the upstream branch.

The upstream branch is rather used when doing `git rebase`. Without an
argument, it rebases to the upstream branch. Likewise, when `git pull`
does the rebase, it'll rebase to the upstream branch. In many cases, if
a PR is rebased, the intention would be to rebase to the trunk branch
(master/main), not the same branch on the remote.

Based on this, it's not necessary for av-cli to modify the branch
upstream. It's not related to git-push (in most of the cases). Leaving
it intact should allow users to choose whichever branch that makes sense
to them (I personally use origin/main because it makes `git rebase`
easy).

This should not affect most of the users. Most of them won't notice
the change. The only case they are affected is (1) They do not use `av
pr create` but use `av stack sync` by some means creating a branch
metadata and (2) set the upstream branch to a different name manually
and (3) pushing to a different name branch manually or setting
"upstream" strategy explicitly in the git config. Again, it's very
unlikely this happens because `av pr create` did follow the "simple"
strategy rule, which is git's default, and in order for a user to be
affected by this change, they somehow need to get around it to use the
"upstream" strategy.
internal/actions/pr.go Outdated Show resolved Hide resolved
Co-authored-by: Travis DePrato <[email protected]>
@aviator-app aviator-app bot merged commit 949593a into master Apr 20, 2023
@aviator-app aviator-app bot deleted the donot_modify_branch_merge branch April 20, 2023 20:01
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