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

Standardize segment typing based on exposure #23315

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

anthony-murphy
Copy link
Contributor

@anthony-murphy anthony-murphy commented Dec 12, 2024

I merge tree we have three different layers of segment:

  1. ISegment - This is the consumer facing type which is exposed via legacy
  2. ISegmentInternal - This type is used internally between fluid framework packages that leverage merge-tree
  3. ISegmentLeaf - This type is not exported from the merge-tree package.

This change standardizes usage in preparation for the upcoming changes which will move some properties between the layers. Doing this change first will reduce the number of lines modified when changes are made.

There are not functional changes in this PR, it is all typing, and there are not changes to any types which are exported to customers, so no type test or api markdowns are impacted

@Copilot Copilot bot review requested due to automatic review settings December 12, 2024 18:35
@anthony-murphy anthony-murphy requested a review from a team as a code owner December 12, 2024 18:35
@github-actions github-actions bot added area: dds Issues related to distributed data structures base: main PRs targeted against main branch labels Dec 12, 2024

Choose a reason for hiding this comment

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

Copilot reviewed 17 out of 32 changed files in this pull request and generated no suggestions.

Files not reviewed (15)
  • packages/dds/merge-tree/src/perspective.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/sortedSegmentSet.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/snapshotlegacy.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/snapshotV1.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/snapshotLoader.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/revertibles.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/partialLengths.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/beastTest.spec.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/client.applyMsg.spec.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/client.localReferenceFarm.spec.ts: Evaluated as low risk
  • packages/dds/matrix/src/matrix.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/client.localReference.spec.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/client.attributionFarm.spec.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/test/client.getPosition.spec.ts: Evaluated as low risk
  • packages/dds/merge-tree/src/MergeTreeTextHelper.ts: Evaluated as low risk
Copy link
Contributor

@jzaffiro jzaffiro left a comment

Choose a reason for hiding this comment

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

One minor comment, but looks good otherwise

packages/dds/merge-tree/src/mergeTreeNodeWalk.ts Outdated Show resolved Hide resolved
@anthony-murphy anthony-murphy enabled auto-merge (squash) December 12, 2024 19:11
@anthony-murphy anthony-murphy merged commit 87f2eef into microsoft:main Dec 12, 2024
26 checks passed
Josmithr pushed a commit to Josmithr/FluidFramework that referenced this pull request Dec 12, 2024
I merge tree we have three different layers of segment:

1. ISegment - This is the consumer facing type which is exposed via
legacy
2. ISegmentInternal - This type is used internally between fluid
framework packages that leverage merge-tree
3. ISegmentLeaf - This type is not exported from the merge-tree package.

This change standardizes usage in preparation for the upcoming changes
which will move some properties between the layers. Doing this change
first will reduce the number of lines modified when changes are made.

There are not functional changes in this PR, it is all typing, and there
are not changes to any types which are exported to customers, so no type
test or api markdowns are impacted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds Issues related to distributed data structures base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants