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

Remove open commits (see discussion #321) #390

Merged
merged 12 commits into from
Jul 1, 2022
Merged

Remove open commits (see discussion #321) #390

merged 12 commits into from
Jul 1, 2022

Conversation

martinvonz
Copy link
Owner

@martinvonz martinvonz commented Jun 23, 2022

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)

`DescendantRebaser::update_checkouts()` only cares about the first
element in `new_commit_ids`, so let's just pass in a single commit ID
instead.
Since we're thinking of removing the concept of open and closed
commits, we can't use that open/closed distinction to decide to not
push some commits.
I'm about to change how `jj checkout` works w.r.t. open/closed commits
(and in particular by moving some logic from the library crate to the
CLI), so let's make sure we have some test coverage.
This is a little refactoring to prepare for removing the `open` flag
off of commits.
I think it's conceptually simpler to create a new commit and set that
commit to be the checkout in each workspace than to check out the
commit in one workspace and edit in the others.
When a commit gets rewritten, we update any workspaces pointing to the
old commit to check out the rewritten commit. If the rewritten commit
is closed, we create a new working-copy commit on top of it. Since
we're thinking about removing the open/closed concept, we need to make
`jj close` manually create the new working-copy commit.
When rebasing commits after rewrites, we also update all workspaces'
checkouts. If the new commit is closed, we create a new commit on
top. Since we're hoping to remove the open/closed concept, we need a
new condition. I considered creating a new commit on top if the change
ID was different from before the rewrite. However, that would make at
least `jj split` more complicated because it makes the first commit
keep the change ID but it wants the second commit to be checked
out. This patch instead creates the new commit on top only when the
original commit was abandoned.
@martinvonz martinvonz marked this pull request as ready for review June 29, 2022 02:05
lib/src/rewrite.rs Outdated Show resolved Hide resolved
src/commands.rs Show resolved Hide resolved
lib/src/settings.rs Outdated Show resolved Hide resolved
By adding `ui.open-commits=false` in your config, you can now make `jj
checkout` always create a new working-copy commit on top of the
specified commit. If the config is set, open commits will also appear
in the same color as closed commits in `jj log` etc. This will let
some of us experiment with the new UX before we decide if it's a good
idea or not. I left `jj close` in place because it's useful for
setting a description and creating a new commit in one step.

I didn't mention the new config in the release notes because I hope we
can reach a decision and remove the config before the next release.
@martinvonz martinvonz merged commit 418ab22 into main Jul 1, 2022
@martinvonz martinvonz deleted the no-open branch July 1, 2022 00:58
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.

None yet

2 participants