squash: support --restore-descendants - #9487
Conversation
There was a problem hiding this comment.
The following commits do not follow our format for subject lines:
- 72f1215: support
--restore-descendantsforsquash
Commits should have a subject line following the format <topic>: <description>. Please review the commit guidelines for more information.
|
Please move the motivation from the PR into the commit description to adhere to our commit style. Also adjust the topic like the bot suggests. |
Sorry, internet went out. Working on it now. |
72f1215 to
a661892
Compare
All commits are now correctly formatted. Thank you for your contribution!
a661892 to
220ca06
Compare
--restore-descendants for squashjj·squash:·support·--restore-descendants
220ca06 to
016b462
Compare
PhilipMetzger
left a comment
There was a problem hiding this comment.
lgtm, still needs maintainer approval though
|
Following https://docs.jj-vcs.dev/latest/contributing/#commit-guidelines, maybe the topic of the commit subject should be Also, there is a missing backtick after |
016b462 to
b3506b2
Compare
jj·squash:·support·--restore-descendants--restore-descendants
Oops, didn't notice that part of the page. (I was there.)
Fixed as well. |
I think it's worth doing that so we don't need to rehash the same things here. |
My implementation differs, so it's not trivial to translate the feedback. I can't reproduce the bugs mentioned in the discussions there. Conflicts work correctly if you squash to a destination where the diff doesn't apply cleanly, the flag works in combination with -A/-B, nothing weird happens when the squash source has descendants, etc. No need to disallow anything. I would really prefer some individual feedback. The only thing that seems potentially relevant is partial squashing into ancestors, but with my implementation that moves the change to exclusively that ancestor and correctly propagates the lack of it to descendants, so I don't see an issue there either. Everything seems to be fairly obvious and there don't seem to be any unintuitive edge cases that I can tell. |
b3506b2 to
aea9739
Compare
aea9739 to
e7fe964
Compare
|
@ilyagr: Any chance you can review this PR? |
d5968eb to
e091a96
Compare
b3d5806 to
583ca7e
Compare
583ca7e to
b6d460e
Compare
b6d460e to
1a96b8c
Compare
1a96b8c to
8386d3f
Compare
|
rebased onto the latest master. (or "main") Do we know if a review is likely to happen? |
|
I'd really like this or #6141 to land, since I've now repeatedly hit cases where it is extremely useful. |
jennings
left a comment
There was a problem hiding this comment.
I did not review the implementation, but the tests seem to show a reasonable behavior. I just added some suggestions for making the tests easier to show how the graph changes.
8386d3f to
89508db
Compare
89508db to
6756772
Compare
Sometimes it is convenient to be able to make changes to a commit in the past without affecting descendants -- I already have a workflow that tracks the commits of all children, makes the changes, and then restores manually, but that performs a lot more rebases than necessary, which adds up when history is long. Supporting `--restore-descendants` for `jj squash` helps this workflow greatly.
6756772 to
169be4e
Compare
Sometimes it is convenient to be able to make changes to a commit in the past without affecting descendants -- I already have a workflow that tracks the commits of all children, makes the changes, and then restores manually, but that performs a lot more rebases than necessary, which adds up when history is long. Supporting
--restore-descendantsforjj squashhelps this workflow greatly.Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)how it works, how it's organized), including any code drafted by an LLM.
an eye towards deleting anything that is irrelevant, clarifying anything
that is confusing, and adding details that are relevant. This includes,
for example, commit descriptions, PR descriptions, and code comments.
Prior Art
I found #6141 but haven't looked too far into it.