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

Tree: Cleanup and document dependencies within tree #11656

Merged
merged 4 commits into from Aug 25, 2022

Conversation

CraigMacomber
Copy link
Contributor

Description

Documents the existing dependency structure, adds a couple missing readmes, and removes some unused deps from the fence files.

Reviewer Guidance

Focus here should be on the readme content and related changes. Feedback on the actual dependency structure (other than the removed deps) is a separate concern beyond this PR.

@CraigMacomber CraigMacomber requested a review from a team as a code owner August 24, 2022 21:50
@github-actions github-actions bot added area: dds Issues related to distributed data structures base: main PRs targeted against main branch labels Aug 24, 2022
packages/dds/tree/README.md Outdated Show resolved Hide resolved
packages/dds/tree/README.md Outdated Show resolved Hide resolved

- Reducing transitive dependencies:

Try to keep the total number of dependencies of a given component small when possible, with a particular emphasis on avoiding stateful dependencies for code with complex conditional logic.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not completely clear to me what is meant by stateful dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rewrote and extended this section. Hopefully its more clear now

packages/dds/tree/README.md Outdated Show resolved Hide resolved
packages/dds/tree/README.md Outdated Show resolved Hide resolved
packages/dds/tree/README.md Outdated Show resolved Hide resolved
packages/dds/tree/README.md Outdated Show resolved Hide resolved
packages/dds/tree/README.md Outdated Show resolved Hide resolved
A change family has to handle all its supported changes identically between all clients, and across all versions.
Support for new kinds of changes can be added, but updating applications to use the new changes must be done carefully to avoid breaking old clients.

Applications that only have ephemeral sessions and never persist documents with trailing ops may be able to relax this constraint somewhat.
Copy link
Contributor

Choose a reason for hiding this comment

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

What are trailing ops?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ops stored in server side summaries which are applied on container load. Thet exist to prevent dataloss when a session ends between summaries. As they are an important concept in fluid, but we don't seem to define them anywhere (though the term is used several times), I'm asking internally where documentation about them belongs and will try and update and link that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like there currently arn't docs covering trailing ops. When such docs get added I'll update this to link them, but for now I'll leave this as is.

CraigMacomber and others added 3 commits August 24, 2022 16:35
Co-authored-by: alex-pardes <105307164+alex-pardes@users.noreply.github.com>
Co-authored-by: alex-pardes <105307164+alex-pardes@users.noreply.github.com>
@CraigMacomber CraigMacomber merged commit b7b2bd3 into microsoft:main Aug 25, 2022
@CraigMacomber CraigMacomber deleted the tree-deps branch August 25, 2022 19:54
@github-actions
Copy link
Contributor

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

tyler-cai-microsoft pushed a commit to tyler-cai-microsoft/FluidFramework that referenced this pull request Aug 26, 2022
WayneFerrao pushed a commit to WayneFerrao/FluidFramework that referenced this pull request Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds Issues related to distributed data structures base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants