Skip to content

Odsp create new module#23049

Merged
anthony-murphy merged 18 commits into
microsoft:mainfrom
anthony-murphy:odsp-create-new-module
Nov 11, 2024
Merged

Odsp create new module#23049
anthony-murphy merged 18 commits into
microsoft:mainfrom
anthony-murphy:odsp-create-new-module

Conversation

@anthony-murphy
Copy link
Copy Markdown
Contributor

@anthony-murphy anthony-murphy commented Nov 11, 2024

This change takes the existing logic to load the createNewModule and encapsulates it in a function to make it more reusable, and less error prone.

The primary motivation for this change is that in an upcoming change I'll want to use the createNewModule and don't want to duplicate the existing logic

By moving the files of the createNewModule under a folder, and not exporting them directly, it makes it clearer that they should not be directly imported.

@github-actions github-actions Bot added area: driver Driver related issues area: odsp-driver base: main PRs targeted against main branch labels Nov 11, 2024
@anthony-murphy anthony-murphy marked this pull request as ready for review November 11, 2024 19:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 7 changed files in this pull request and generated no suggestions.

Files not reviewed (2)
  • packages/drivers/odsp-driver/src/odspDocumentServiceFactoryCore.ts: Evaluated as low risk
  • packages/drivers/odsp-driver/src/createFile/createNewUtils.ts: Evaluated as low risk

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

Copy link
Copy Markdown
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

↑ packages.drivers.odsp-driver.src:
Line Coverage Change: -0.78%    Branch Coverage Change: 0.94%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 85.33% 86.27% ↑ 0.94%
Line Coverage 74.90% 74.12% ↓ -0.78%
↑ packages.drivers.odsp-driver.src.createFile:
Line Coverage Change: 88.34%    Branch Coverage Change: 71.92%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 0.00% 71.92% ↑ 71.92%
Line Coverage 0.00% 88.34% ↑ 88.34%
↑ packages.drivers.odsp-driver.src.createFile:
Line Coverage Change: 88.34%    Branch Coverage Change: 71.92%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 0.00% 71.92% ↑ 71.92%
Line Coverage 0.00% 88.34% ↑ 88.34%

Baseline commit: 3c27751
Baseline build: 305767
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Copy Markdown
Collaborator

msfluid-bot commented Nov 11, 2024

@fluid-example/bundle-size-tests: +373 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 463.74 KB 463.77 KB +35 Bytes
azureClient.js 562.69 KB 562.74 KB +49 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 261.99 KB 262 KB +14 Bytes
fluidFramework.js 426.65 KB 426.66 KB +14 Bytes
loader.js 134.18 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 148.32 KB 148.33 KB +7 Bytes
odspClient.js 528.43 KB 528.53 KB +96 Bytes
odspDriver.js 97.84 KB 97.9 KB +58 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 164.17 KB 164.17 KB +7 Bytes
sharedTree.js 417.11 KB 417.11 KB +7 Bytes
Total Size 3.36 MB 3.36 MB +373 Bytes

Baseline commit: 3c27751

Generated by 🚫 dangerJS against 7f6db69

Comment thread packages/drivers/odsp-driver/src/createFile/index.ts Outdated
@anthony-murphy anthony-murphy enabled auto-merge (squash) November 11, 2024 20:59
@anthony-murphy anthony-murphy merged commit 5156eb4 into microsoft:main Nov 11, 2024
@anthony-murphy anthony-murphy deleted the odsp-create-new-module branch November 11, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: driver Driver related issues area: odsp-driver base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants