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

remove snapshot() path from Container #3929

Closed
Tracked by #4683
vladsud opened this issue Oct 13, 2020 · 7 comments
Closed
Tracked by #4683

remove snapshot() path from Container #3929

vladsud opened this issue Oct 13, 2020 · 7 comments
Assignees
Labels
api design-required This issue requires design thought refactor Code refactoring and cleanup
Milestone

Comments

@vladsud
Copy link
Contributor

vladsud commented Oct 13, 2020

Only summaries should be used.
There are some places (DDS) where we still use "snapshot" terminology & types - we should change that to use summaries.

As part of it, we should remove / move to r11 driver(s) all GIT specifics like FileMode and remove them from protocol-defintions.

@vladsud vladsud added the bug Something isn't working label Oct 13, 2020
@vladsud vladsud added this to the November 2020 milestone Oct 13, 2020
@ghost ghost added the triage label Oct 13, 2020
@vladsud
Copy link
Contributor Author

vladsud commented Oct 13, 2020

@arinwt - FYI

@vladsud vladsud self-assigned this Oct 13, 2020
@vladsud
Copy link
Contributor Author

vladsud commented Oct 13, 2020

I'm sure there are duplicates of this item :)
The most challenging part here would be fixing tooling, i.e. replay tool. It's a must to maintain it (tests would fail otherwise). We either need to move it to summaries, or do a conversion somewhere for it to still get snapshot when runtime talks summaries.

@curtisman curtisman added api refactor Code refactoring and cleanup and removed bug Something isn't working triage labels Oct 20, 2020
@curtisman curtisman assigned agarwal-navin and unassigned jatgarg Oct 30, 2020
@vladsud vladsud removed their assignment Nov 12, 2020
@curtisman curtisman modified the milestones: February 2021, Next Jan 28, 2021
@ghost ghost added the status: stale label Jul 28, 2021
@ghost
Copy link

ghost commented Jul 28, 2021

This issue has been automatically marked as stale because it has had no activity for 180 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!

@danielroney danielroney added epic An issue that is a parent to several other issues design-required This issue requires design thought labels Oct 27, 2021
@nmsimons nmsimons removed the epic An issue that is a parent to several other issues label Oct 27, 2021
@vladsud
Copy link
Contributor Author

vladsud commented Oct 27, 2021

There is Container.snapshot() that needs to be removed.
There is also SharedObject.snapshotCore() and a bunch of implementations on derived classes.

Let's keep this item to track Container work, which is lower priority than #8013 that I created to track the more interesting part

@vladsud vladsud changed the title We need to remove snapshot() path from runtime remove snapshot() path from Container Oct 27, 2021
@ssimic2
Copy link

ssimic2 commented Feb 3, 2022

Given we have completed some of the work around old snapshots, it may be good to time redefine scope of this ticket. And close it if needed.

@agarwal-navin
Copy link
Contributor

Adding @scarlettjlee to this since she is working on related topic - #8460

@scarlettjlee
Copy link
Contributor

The DDS work is done and I created a separate task for moving git code from protocol-definitions (#9003). So this issue should just be for removing snapshot from Container.

@ssimic2, do you want to keep this open until you do the removal after the deprecation? Otherwise we can close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api design-required This issue requires design thought refactor Code refactoring and cleanup
Projects
None yet
Development

No branches or pull requests

8 participants