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
build: Update changesets and add release notes #17512
build: Update changesets and add release notes #17512
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that threw me off below. And I don't see the new release notes file mentioned in the description?
| @@ -1,8 +1,8 @@ | |||
| --- | |||
| "@fluidframework/container-utils": major | |||
| "@fluidframework/telemetry-utils": major | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't have a changeset that refers to a package that doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since most of the functions went to telemetry-tuils, I used that package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm leaves me wondering how do we announce "this package is now completely gone and there are no replacements for its exports" if we ever need to. And in fact I think it kind of applies here? The things that moved over, I imagine might not be intended for use by external consumers, and this makes it feel like if they were using them, they should now just use them from telemetry-utils. Disclaimer: I didn't go check what moved, so might be wrong on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaves me wondering how do we announce "this package is now completely gone and there are no replacements for its exports" if we ever need to
I'm not worried about that. In that case we don't really need to announce the package is gone. The last version of the package published will have all the APIs deprecated (e.g. we just did this with common-utils), and once deleted, there's just never a new version published. That's basically what happened here - we shipped deprecations in 6.2.0/6.3.0 (I assume we announced the deprecations in the release notes then), and that's the "final" version of that package.
In other words, the decision of replacement APIs and announcing them really needs to be made when deprecating them, not when removing them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put in the body another sentence or phrase "Many of them were moved to telemetry-utils" or whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe you guys are missing the whole changeset content, which is:
All members of the @fluidframework/container-utils package have been deprecated and the package is now removed.
Migration by API member:
ClientSessionExpiredError(deprecated in2.0.0-internal.6.2.0): No replacement API offered.DataCorruptionError(deprecated in2.0.0-internal.6.2.0): Import from @fluidframework/telemetry-utils instead.DataProcessingError(deprecated in2.0.0-internal.6.2.0): Import from @fluidframework/telemetry-utils instead.DeltaManagerProxyBase(deprecated in2.0.0-internal.6.1.0): No replacement API offered.extractSafePropertiesFromMessage(deprecated in2.0.0-internal.6.2.0): Import from @fluidframework/telemetry-utils instead.GenericError(deprecated in2.0.0-internal.6.2.0): Import from @fluidframework/telemetry-utils instead.ThrottlingWarning(deprecated in2.0.0-internal.6.2.0): No replacement API offered.UsageError(deprecated in2.0.0-internal.6.2.0): Import from @fluidframework/telemetry-utils instead.
I think that's pretty thorough.
Sorry about that - they should be there now. |
Co-authored-by: Mark Fields <markfields@users.noreply.github.com>
Co-authored-by: Mark Fields <markfields@users.noreply.github.com>
| @@ -2,6 +2,6 @@ | |||
| "@fluidframework/container-loader": major | |||
| --- | |||
|
|
|||
| Container caching in the Loader is removed | |||
| container-loader: Container caching in the Loader is removed | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexvy86 - Can you follow up with stakeholders on FF/Loop here to make sure this won't end up blocking the bump? It sounded like the migration was a little disjointed based on the firedrill that happened during the last major bump.
Updates the changesets for clarity and adds a release notes file that will be pasted into the GitHub release.