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.

Sign up for GitHub

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

Jump to bottom

index: IndexSegment API cleanup #2743

Merged
merged 5 commits into from
Dec 25, 2023
Merged

index: IndexSegment API cleanup #2743

merged 5 commits into from
Dec 25, 2023

Conversation

yuja
Copy link
Contributor

@yuja yuja commented Dec 25, 2023

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

yuja added 5 commits December 25, 2023 23:31
@yuja
index: inline segment_entry_by_pos()
7c3e775 
There's no reasonable way to abstract the IndexEntry construction.
@yuja
index: make segment-level lookup return neighbor commit ids instead o…
dee463b 
…f positions

Both readonly and mutable segments know the commit ids to return, and the
caller only needs the ids. Since segment_commit_id(local_pos) scans the graph
entries, doing that would increase the chance of cache miss.
@yuja
index: introduce newtype that represents segment-local position
0215a4a 
I'm thinking of changing some IndexSegment methods to return LocalPosition
instead of global IndexPosition, and using u32 there would be a source of bugs.
@yuja
index: remove "segment_" prefix from IndexSegment methods
2d82025 
Since Readonly/MutableIndexSegment no longer implement Index trait, there's
no ambiguity between segment-local and index-global operations. Let's shorten
the method names.
@yuja
index: rename resolve_prefix() to resolve_commit_id_prefix()
5b401a2 
I'll probably add change id lookup methods to CompositeIndex. The Index trait
won't gain resolve_change_id_prefix(), but I also renamed its resolve_prefix()
for consistency.
@yuja yuja force-pushed the push-trmzzstykoum branch from 559a97b to 5b401a2 Compare December 25, 2023 15:06
@yuja yuja merged commit dde42b9 into jj-vcs:main Dec 25, 2023
15 checks passed
@yuja yuja deleted the push-trmzzstykoum branch December 25, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinvonz martinvonz martinvonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yuja @martinvonz