Refactor MutRepo to make DescendantRebaser private (pt 1) #2737
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ilyagr
force-pushed
the
rm-descreb-api-pt1
branch
2 times, most recently
from
9d63232 to
d27cceb
Compare
1 task
ilyagr
force-pushed
the
rm-descreb-api-pt1
branch
from
d27cceb to
0671d5e
Compare
yuja
reviewed
Dec 23, 2023
The implementation of `CommitBuilder::write` is tightly bound to the MutRepo, so only MutRepo should construct CommitBuilder-s.
After this, the internal function is only used in tests.
At some point, I tried `new_commit` instead of `rewrite_commit` in the split command. That seemed to work, but messed up the dates in a subtle way. This commit should prevents repeats of this mistake and emphasize the importance of the author dates being preserved.
ilyagr
force-pushed
the
rm-descreb-api-pt1
branch
2 times, most recently
from
e82d7bc to
8a10afa
Compare
ilyagr
commented
Dec 24, 2023
yuja
approved these changes
Dec 24, 2023
martinvonz
reviewed
Dec 24, 2023
This requires creating a new public API as a substitute. I took the opportunity to also add some comments to the `MutRepo::record_rewritten_commit`/`record_abandoned_commit` functions. I imade the simplest possible addition to the API; it is not a very elegant one. Eventually, the entire `record_rewritten_commit` API should probably be refactored again. I also added some comments explaining what these functions do.
ilyagr
force-pushed
the
rm-descreb-api-pt1
branch
from
8a10afa to
8e8bcaf
Compare
|
I'll merge this and mark next PR as ready. We can clean anything up there or in follow-ups if necessary. |
ilyagr
added a commit
that referenced
this pull request
Jan 2, 2024
…mmits This commit is a little out of place in this sequence, but it seems to make more sense for MutRepo to own these maps. @yuja [pointed out] that any tests written using `create_descendant_rebaser` now need to do this cleanup, but there are no longer any such tests after the previous commits and a follow-up commit removes `create_descendant_rebaser` entirely. [pointed out]: #2737 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After this PR, the soon-to-be-internal APIs are used only in tests. The second part of this will be #2738.
The goal is to be able to more freely refactor DescendantRebaser after this. The new public API is not necessarily final or perfect, but should already be more manageable.
Checklist
If applicable:
CHANGELOG.md