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

Run lint:fix on Container Runtime to remove suprious deprecations #24022

Merged
merged 1 commit into from
Mar 11, 2025
Merged

Run lint:fix on Container Runtime to remove suprious deprecations #24022

merged 1 commit into from
Mar 11, 2025

Conversation

anthony-murphy
Copy link
Contributor

Just ran link fix to remove any spurious deprecation lines, including those created by #23981

@Copilot Copilot bot review requested due to automatic review settings March 11, 2025 01:37
@github-actions github-actions bot added area: runtime Runtime related issues base: main PRs targeted against main branch labels Mar 11, 2025
@anthony-murphy anthony-murphy mentioned this pull request Mar 11, 2025
Merged
Copilot
Copilot AI reviewed Mar 11, 2025
View reviewed changes

Choose a reason for hiding this comment

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

PR Overview

This PR applies a lint fix to remove several spurious deprecation comments throughout the Container Runtime code. The key changes include the removal of redundant “eslint-disable-next-line import/no-deprecated” comments from various files and adjustments to the surrounding whitespace.

Reviewed Changes

File Description
packages/runtime/container-runtime/src/gc/gcConfigs.ts Removed redundant deprecation directives from import statements.
packages/runtime/container-runtime/src/summary/summaryFormat.ts Cleaned up deprecation comments alongside metadata-related exports.
packages/runtime/container-runtime/src/containerRuntime.ts Removed several deprecation comments from core runtime code, with minor whitespace changes.

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)
jason-ha
jason-ha approved these changes Mar 11, 2025
View reviewed changes
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.

Too bad lint:fix leaves a bunch of blank lines in funny places and nothing will fix 'em up.
Wonder if the better process is to remove all of them from a package and then let lint:fix restore the one that are needed.

packages/runtime/container-runtime/src/blobManager/blobManager.ts
Comment on lines -189 to +188
// eslint-disable-next-line import/no-deprecated

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if most or all of these were just deletions

packages/runtime/container-runtime/src/channelCollection.ts
Comment on lines -1490 to +1486
// eslint-disable-next-line import/no-deprecated

Copy link
Contributor

Choose a reason for hiding this comment

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

This gap is definitely funny.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah. its just want es-lint auto fix does. i don't really like it, but i also not keen on doing a bunch of manual clean up

@anthony-murphy
Copy link
Contributor Author

Too bad lint:fix leaves a bunch of blank lines in funny places and nothing will fix 'em up. Wonder if the better process is to remove all of them from a package and then let lint:fix restore the one that are needed.

it looks like es-lint doesn't know how to add inline suppressions, or i don't know how to do it, so it would again require a bunch of manual fix ups.

@anthony-murphy anthony-murphy merged commit 92e911d into microsoft:main Mar 11, 2025
35 checks passed
@anthony-murphy anthony-murphy deleted the cr-lint-fix branch March 11, 2025 16:26
@jason-ha
Copy link
Contributor

jason-ha commented Mar 11, 2025 •
edited
Loading

Yeah - I don't know how to apply disables other than IDE option. Makes some sense because it can't be "fixed". Maybe the more recent versions of linter do a better job.
Oh, and I think Loop has a script that they use to apply disables. Maybe an option if we have a big clean up in the future.
Looks like there are only 5 deprecation uses remaining in container-runtime. So those could have been manually restored, but not a process I would want to recommend generally until we have a lot fewer deprecated APIs in use.

I am guessing these no longer need to be legacy+alpha:
ReadFluidDataStoreAttributes
IFluidDataStoreAttributes0
IFluidDataStoreAttributes1
IFluidDataStoreAttributes2

At least I don't see hits for them in any other *.api.md files.

chentong7 pushed a commit to chentong7/FluidFramework that referenced this pull request Mar 11, 2025
@anthony-murphy @chentong7
Run lint:fix on Container Runtime to remove suprious deprecations (mi…
1ee5907 
…crosoft#24022)

Just ran link fix to remove any spurious deprecation lines, including
those created by microsoft#23981
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@jason-ha jason-ha jason-ha approved these changes

Assignees
No one assigned
Labels
area: runtime Runtime related issues base: main PRs targeted against main branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@anthony-murphy @jason-ha