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

[For 2.20] Remove IContainerRuntimeOptions.flushmode #23337

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[For 2.20] Remove IContainerRuntimeOptions.flushmode #23337

wants to merge 2 commits into from
+14 −11

Conversation

markfields
Copy link
Member

@markfields markfields commented Dec 16, 2024 •
edited by azure-boards bot
Loading

Description

Fixes AB#26496
See #23287

IContainerRuntimeOptions.flushMode is being removed

This option allowed an application specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"
(batched by JS turn). But Immediate mode has been deprecated and should no longer be used.

Now there is only one choice, which is the default TurnBased mode. So we can simply remove this option.

Breaking Changes

This is a breaking change to the IContainerRuntimeOptions interface. flushMode is deprecated as of 2.12.0, and there are no known usages in partner codebases. FlushMode.Immediate has been deprecated since 2.0.0-rc.2.0.0.

@Copilot Copilot bot review requested due to automatic review settings December 16, 2024 23:06
@markfields markfields requested review from a team as code owners December 16, 2024 23:06
@github-actions github-actions bot added area: runtime Runtime related issues changeset-present public api change Changes to a public API labels Dec 16, 2024
Copilot
Copilot AI reviewed Dec 16, 2024
View reviewed changes

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
@markfields markfields marked this pull request as draft December 16, 2024 23:07
@github-actions github-actions bot added the base: main PRs targeted against main branch label Dec 16, 2024
@markfields markfields linked an issue Dec 16, 2024 that may be closed by this pull request
Client 2.20 Legacy-Alpha Breaking Changes #22835
Open
@markfields markfields requested review from jason-ha, kian-thompson, MarioJGMsoft, a team, pragya91, jatgarg, tyler-cai-microsoft and rajatch-ff and removed request for a team December 16, 2024 23:15
@github-actions GitHub Actions
Copy link
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170006 links
    1595 destination URLs
    1825 URLs ignored
       0 warnings
       0 errors

msfluid-bot
msfluid-bot reviewed Dec 17, 2024
View reviewed changes
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

No packages impacted by the change.

Baseline commit: 35e9227
Baseline build: 312904
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

⯅ @fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 469.94 KB 469.97 KB ⯅ +35 Bytes
azureClient.js 567 KB 567.05 KB ⯅ +49 Bytes
connectionState.js 724 Bytes 724 Bytes ■ No change
containerRuntime.js 266.15 KB 266.16 KB ⯅ +14 Bytes
fluidFramework.js 434.72 KB 434.73 KB ⯅ +14 Bytes
loader.js 134.22 KB 134.23 KB ⯅ +14 Bytes
map.js 42.68 KB 42.69 KB ⯅ +7 Bytes
matrix.js 150.13 KB 150.13 KB ⯅ +7 Bytes
odspClient.js 534.46 KB 534.51 KB ⯅ +49 Bytes
odspDriver.js 99.48 KB 99.5 KB ⯅ +21 Bytes
odspPrefetchSnapshot.js 43.04 KB 43.05 KB ⯅ +14 Bytes
sharedString.js 166.21 KB 166.21 KB ⯅ +7 Bytes
sharedTree.js 425.19 KB 425.2 KB ⯅ +7 Bytes
Total Size 3.41 MB 3.41 MB ⯅ +245 Bytes

Baseline commit: 35e9227

Generated by 🚫 dangerJS against 1e81733

jzaffiro
jzaffiro approved these changes Dec 17, 2024
View reviewed changes
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.

A few minor tweaks to wording, but docs look good!

.changeset/cruel-horses-cheat.md
IContainerRuntimeOptions.flushMode has been removed

This option allowed an application specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"
(batched by JS turn). But `Immediate` mode has been deprecated and should no longer be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(batched by JS turn). But `Immediate` mode has been deprecated and should no longer be used.
(batched by JS turn). `Immediate` mode has been deprecated and should no longer be used.

.changeset/cruel-horses-cheat.md

IContainerRuntimeOptions.flushMode has been removed

This option allowed an application specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This option allowed an application specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"
This option allowed an application to specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"

.changeset/cruel-horses-cheat.md
This option allowed an application specify whether to flush ops "immediately" (literally 1-by-1) or "turn-based"
(batched by JS turn). But `Immediate` mode has been deprecated and should no longer be used.

Now there is only one choice, which is the default `TurnBased` mode. So we can simply remove this option.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Now there is only one choice, which is the default `TurnBased` mode. So we can simply remove this option.
Now there is only one choice, which is the default `TurnBased` mode, so the `Immediate` mode can be removed.

@markfields markfields linked an issue Dec 17, 2024 that may be closed by this pull request
Remove IContainerRuntimeOptions.flushMode #23287
Open
@jason-ha jason-ha removed a link to an issue Dec 17, 2024
Client 2.20 Legacy-Alpha Breaking Changes #22835
Open
jason-ha
jason-ha approved these changes Dec 17, 2024
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.

I think it would also be okay to point to deprecation release notes (when available)

@markfields
Copy link
Member Author

I think it would also be okay to point to deprecation release notes (when available)

@jason-ha what specifically are you suggesting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot Copilot left review comments

@msfluid-bot msfluid-bot msfluid-bot left review comments

@jzaffiro jzaffiro jzaffiro approved these changes

@kian-thompson kian-thompson kian-thompson approved these changes

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

@MarioJGMsoft MarioJGMsoft Awaiting requested review from MarioJGMsoft

@pragya91 pragya91 Awaiting requested review from pragya91 pragya91 was automatically assigned from microsoft/team-rajat

@jatgarg jatgarg Awaiting requested review from jatgarg jatgarg was automatically assigned from microsoft/team-rajat

@tyler-cai-microsoft tyler-cai-microsoft Awaiting requested review from tyler-cai-microsoft tyler-cai-microsoft was automatically assigned from microsoft/team-rajat

@rajatch-ff rajatch-ff Awaiting requested review from rajatch-ff rajatch-ff was automatically assigned from microsoft/team-rajat

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area: runtime Runtime related issues base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove IContainerRuntimeOptions.flushMode
5 participants
@markfields @msfluid-bot @jason-ha @kian-thompson @jzaffiro