cli: change the default revsets for push to be based on trunk() #2263
Conversation
There was a problem hiding this comment.
I'm not sure whether this is always an improvement.
The situation this is meant for, as far as I understand, is if a descendant of @ or of @- has branches on it. Currently, jj git push wouldn't push those, which may or may not be intuitive.
One argument for the current behavior is that jj co <some-rev>; jj git push allows for a somewhat surgical pushing, if the work on the descendants is too incomplete to share.
Another possible problem with the approach of this PR is if a tree has a huge side branch (* represent commits, the root of the tree is on the left):
-----*------------------------* (trunk)
\
----*---*---*----*------* @
\
--*--*----* (other stuff)
\ \
* *
With this PR, jj git push would push all the branches of (other stuff) as well. I'm not sure how common such a situation is, but it feels mildly worrying.
OTOH, I might not be the best person to judge this since I almost always use jj git push --all.
| @@ -126,7 +126,7 @@ pub struct GitCloneArgs { | |||
| /// Push to a Git remote | |||
| /// | |||
| /// By default, pushes any branches pointing to | |||
| /// `remote_branches(remote=<remote>)..@`. Use `--branch` to push specific | |||
| /// `(trunk()..@):: & branches()`. Use `--branch` to push specific | |||
There was a problem hiding this comment.
The & branches() seems to be unnecessary because of how jj git push -r is defined.
|
OTOH, an argument for something like this is that then the default "push"-set will match the default log revset. |
|
Tabling this until Either way, I think a good approach would be to ensure that the default log, push, and rebase are all in sync. Especially if we want to draw a distinction between rebase vs merge workflows? For example, if we're in rebase/stack world, given a stack with 2 commits on main if this is the default log: I suspect we'd want the default push to be |
As with #2088, I'm not wedded to this, but it seems like a valuable change for my workflow at least and matches git-branchless submit.
Builds on the trunk alias from #2088. The benefits of this over the previous default in my opinion is:
@, meaning you can run it from inside a stack; andtrunk()..@, so if the trunk branch was accidentally modified, the default push won't include it.Note: Once
trunk()is made into a built-in function, we can create astack()function that resolves to(trunk()..@)::which should be a bit cleaner than the current approach.