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

ContainerRuntime: Remove the contents property of batchBegin/batchEnd's "op" event arg #22791

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

markfields
Copy link
Member

@markfields markfields commented Oct 11, 2024

Description

Fixes AB#19784

ContainerRuntime's 'batchBegin'/'batchEnd' events: Removing the contents property on event arg op

The 'batchBegin'/'batchEnd' events on ContainerRuntime indicate when a batch is beginning/finishing being processed.
The contents property on there is not useful or relevant when reasoning over incoming changes at the batch level.
So it has been removed from the op event arg.

Breaking Changes

Yes this is a breaking change. See changeset.

@markfields markfields requested a review from a team as a code owner October 11, 2024 23:32
@github-actions github-actions bot added area: runtime Runtime related issues changeset-present base: main PRs targeted against main branch labels Oct 11, 2024
@markfields markfields marked this pull request as draft October 11, 2024 23:32
@markfields
Copy link
Member Author

Converting to Draft until main is open for breaking changes

@markfields
Copy link
Member Author

I'll be OOF until 11/14. I'll plan to bring this in for 2.10.0 at that point if it's not too late (unless someone decides to merge this for me while I'm OOF)

@github-actions github-actions bot added the public api change Changes to a public API label Oct 11, 2024
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Oct 12, 2024

@fluid-example/bundle-size-tests: +233 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 464.03 KB 464.06 KB +32 Bytes
azureClient.js 562.45 KB 562.49 KB +46 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 261.86 KB 261.87 KB +11 Bytes
fluidFramework.js 424.82 KB 424.84 KB +14 Bytes
loader.js 134.17 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 148.54 KB 148.55 KB +7 Bytes
odspClient.js 528.3 KB 528.34 KB +46 Bytes
odspDriver.js 97.84 KB 97.86 KB +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 164.58 KB 164.58 KB +7 Bytes
sharedTree.js 415.28 KB 415.29 KB +7 Bytes
Total Size 3.36 MB 3.36 MB +233 Bytes

Baseline commit: 2aa0b5e

Generated by 🚫 dangerJS against 5738a9b

@kian-thompson kian-thompson enabled auto-merge (squash) November 5, 2024 23:22
@markfields
Copy link
Member Author

@kian-thompson feel free to merge this if the time is right for breaking changes. Not paying attention broadly, just saw the notification that you approved it.

Copy link
Contributor

github-actions bot commented Nov 5, 2024

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> [email protected] ci:start
> http-server ./public --port 1313 --silent


> [email protected] linkcheck:full
> npm run linkcheck:fast -- --external


> [email protected] linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

Stats:
  447786 links
    3441 destination URLs
       2 URLs ignored
       0 warnings
       0 errors


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: 2aa0b5e
Baseline build: 304363
Happy Coding!!

Code coverage comparison check passed!!

Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

reworded the changeset a bit - approving for docs.

.changeset/fruity-sites-boil.md Show resolved Hide resolved
@kian-thompson kian-thompson merged commit d252af5 into microsoft:main Nov 6, 2024
34 checks passed
kian-thompson added a commit that referenced this pull request Nov 6, 2024
PR #22791 had auto-merge turned on, so a suggested change to the
changeset wasn't made.
#22791 (comment)
@markfields markfields deleted the batch-events-no-contents branch November 7, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

ContainerRuntime: Remove the contents property of batchBegin/batchEnd's "op" event arg
5 participants