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

Removed deprecated usages of the option summarizeProtocolTree #23355

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pragya91
Copy link
Contributor

@pragya91 pragya91 commented Dec 18, 2024

Removing deprecated code that usage of summarizeProtocolTree option from the code. The change is non-breaking in nature. The PR only removes the old unused usages of summarizeProtocolTree.

@github-actions github-actions bot added area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Dec 18, 2024
@pragya91 pragya91 force-pushed the praggarg/fix-imports branch from 6674943 to 7332a96 Compare December 20, 2024 04:44
Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

  1. Looks like @fluidframework/container-loader's version of ILoaderOptions should be removed as well.
  2. Recommend review of summarizeProtocolTree hits in code base as well to confirm there are no dangling oddities.
  3. This will need to wait until 2.30 to happen as the related issue was not in the 2.20 break list. I updated issue to be in the 2.30 list.
  4. Create a branch under main repo under test/legacy-breaks/client/2.30/ according to legacy break process.
  5. Update the legacy break issue to be associated with replacement PR (test/ branch based) and assuming also removing extra ILoaderOptions per (1), then update issue title and content (recommend following Breaking Change template for content).

@@ -28,6 +28,7 @@ import {
isFluidCodeDetails,
IDeltaManager,
ReadOnlyInfo,
ILoaderOptions,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should update to a type only import while touching and respect more alphabetizing (most entries are sorted; just the last few are not)

Comment on lines 127 to 128
// eslint-disable-next-line import/no-deprecated
import { IDetachedBlobStorage, ILoaderOptions, RelativeLoader } from "./loader.js";
import { IDetachedBlobStorage, RelativeLoader } from "./loader.js";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please reformat so that the lint disable applies per import rather than for all imports? Thanks!

@pragya91 pragya91 force-pushed the praggarg/fix-imports branch from 7332a96 to 73a5b97 Compare January 2, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants