Skip to content

cli: push new non-tracking bookmarks by default, rename --allow-new flag - #5173

Closed
yuja wants to merge 1 commit into
jj-vcs:mainfrom
yuja:push-vvvzpmtstnpw
Closed

cli: push new non-tracking bookmarks by default, rename --allow-new flag#5173
yuja wants to merge 1 commit into
jj-vcs:mainfrom
yuja:push-vvvzpmtstnpw

Conversation

@yuja

@yuja yuja commented Dec 22, 2024

Copy link
Copy Markdown
Contributor

#5094

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

This partially reverts 296961c "cli: git push: do not push new bookmarks by
default." Apparently, the restriction introduced by that patch was too strict
for some use cases. The new rule is that any non-tracking (or local-only)
bookmarks will be pushed to new remote by default, but --allow-new-tracking is
required if a bookmark is already tracking other (real) remotes.

Closes jj-vcs#5094
@istudyatuni

Copy link
Copy Markdown

Will it be possible to not push some bookmarks without needing to specify -b [bookmark]?

@yuja

yuja commented Dec 25, 2024

Copy link
Copy Markdown
Contributor Author

Will it be possible to not push some bookmarks without needing to specify -b [bookmark]?

New non-tracking bookmarks within the default push revsets? no.

If you like the current --allow-new behavior, please speak up in #5094. (I personally don't feel --allow-new was a mistake, but appears that people don't like it.)

@martinvonz martinvonz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@yuja

yuja commented Dec 28, 2024

Copy link
Copy Markdown
Contributor Author

I'll leave this open for a while because of #5094 (comment). I don't want to introduce breaking change again.

@martinvonz

Copy link
Copy Markdown
Contributor

I'll leave this open for a while because of #5094 (comment). I don't want to introduce breaking change again.

Makes sense.

I think we're too close to the release to merge this now. The release is planned for Wednesday, but I'm not sure I'll get around to it since I'm on vacation.

yuja added a commit to yuja/jj that referenced this pull request Jan 14, 2025
…y default

This goes against our rule that we shouldn't add config knob that changes the
command behavior, but I don't have any other idea to work around the problem.
Apparently, there are two parties, one who always wants to push new bookmarks,
and the other who mildly prefers to push&track new bookmarks explicitly.
Perhaps, for the former, creation of bookmarks means that the target branches
are marked to be pushed.

The added flag is a simple boolean. "non-tracking-only" behavior jj-vcs#5173 could be
implemented, but I don't want to complicate things. It's a failed attempt to
address the issue without introducing config knob.

Closes jj-vcs#5094
Closes jj-vcs#5173
github-merge-queue Bot pushed a commit that referenced this pull request Jan 15, 2025
…y default

This goes against our rule that we shouldn't add config knob that changes the
command behavior, but I don't have any other idea to work around the problem.
Apparently, there are two parties, one who always wants to push new bookmarks,
and the other who mildly prefers to push&track new bookmarks explicitly.
Perhaps, for the former, creation of bookmarks means that the target branches
are marked to be pushed.

The added flag is a simple boolean. "non-tracking-only" behavior #5173 could be
implemented, but I don't want to complicate things. It's a failed attempt to
address the issue without introducing config knob.

Closes #5094
Closes #5173
@yuja

yuja commented Jan 15, 2025

Copy link
Copy Markdown
Contributor Author

Superseded by f9906dc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants