Deprecate ISummaryAuthor and ISummaryCommitter #10932

ghoshkaj · 2022-06-30T22:33:56Z

Description

Deprecate ISummaryAuthor and ISummaryCommitter as they have never been used. See #10456 for more details.

Does this introduce a breaking change?

Yes, technically, public interfaces are being deprecated but since they have never been used in practicality it should not affect users.

ghoshkaj · 2022-06-30T22:47:10Z

@NicholasCouri it looks like I need to manually rebuild the docs? Is that right? https://dev.azure.com/fluidframework/public/_build/results?buildId=77806&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3&t=5cfe65a4-d7cb-5934-e953-1c67617821f4

anthony-murphy · 2022-07-01T00:48:27Z

yeah. you just need to build which will rebuild the docs and you commit them to your pr branch

NicholasCouri · 2022-07-01T01:17:07Z

1. yes, that is what I shared with her 🙂 - the problem is that she is facing problems building using codespaces - and I'm not familiar with it .

tylerbutler

In the deprecation comments, I would just say "foo is deprecated and will be removed in a future release." Also check the spelling on deprecated. I think there were a couple of others.

BREAKING.md

tylerbutler · 2022-07-01T05:57:23Z

common/lib/protocol-definitions/src/summary.ts

@@ -7,13 +7,21 @@ export type SummaryObject = ISummaryTree | ISummaryBlob | ISummaryHandle | ISumm

 export type SummaryTree = ISummaryTree | ISummaryHandle;

+/**
+ * @deprecated - ISummaryAuthor has never been used anywhere and is unlikely to be used in the


No dash when using the deprecated TSDoc tag. You should also get a lint error with this as it is. Please let me and @Josmithr know if you didn't, because it may be a bug in our lint config.

this isn't causing a lint failure. the pr build passes

It didn't complain about the dash. This is all the output:

@ghoshkaj ➜ .../FluidFramework/common/lib/protocol-definitions (depracate-ISummaryAuthor-ISummaryCommitter) $ npm run build > @fluidframework/[email protected] build /workspaces/FluidFramework/common/lib/protocol-definitions > concurrently npm:build:compile npm:lint && npm run build:docs [build:compile] [build:compile] > @fluidframework/[email protected] build:compile /workspaces/FluidFramework/common/lib/protocol-definitions [build:compile] > concurrently npm:typetests:gen npm:tsc npm:build:esnext [build:compile] [lint] [lint] > @fluidframework/[email protected] lint /workspaces/FluidFramework/common/lib/protocol-definitions [lint] > npm run eslint [lint] [lint] [lint] > @fluidframework/[email protected] eslint /workspaces/FluidFramework/common/lib/protocol-definitions [lint] > eslint --format stylish src [lint] [build:compile] [typetests:gen] [build:compile] [typetests:gen] > @fluidframework/[email protected] typetests:gen /workspaces/FluidFramework/common/lib/protocol-definitions [build:compile] [typetests:gen] > fluid-type-validator -d . [build:compile] [typetests:gen] [build:compile] [build:esnext] [build:compile] [build:esnext] > @fluidframework/[email protected] build:esnext /workspaces/FluidFramework/common/lib/protocol-definitions [build:compile] [build:esnext] > tsc --project ./tsconfig.esnext.json [build:compile] [build:esnext] [build:compile] [tsc] [build:compile] [tsc] > @fluidframework/[email protected] tsc /workspaces/FluidFramework/common/lib/protocol-definitions [build:compile] [tsc] > tsc [build:compile] [tsc] [build:compile] [typetests:gen] npm run typetests:gen exited with code 0 [build:compile] [tsc] npm run tsc exited with code 0 [build:compile] [build:esnext] npm run build:esnext exited with code 0 [build:compile] npm run build:compile exited with code 0 [lint] [lint] /workspaces/FluidFramework/common/lib/protocol-definitions/src/protocol.ts [lint] 181:24 warning Usage of "null" is deprecated except when describing legacy APIs; use "undefined" instead @rushstack/no-new-null [lint] [lint] /workspaces/FluidFramework/common/lib/protocol-definitions/src/sockets.ts [lint] 28:21 warning Usage of "null" is deprecated except when describing legacy APIs; use "undefined" instead @rushstack/no-new-null [lint] [lint] /workspaces/FluidFramework/common/lib/protocol-definitions/src/summary.ts [lint] 35:1 warning Unused eslint-disable directive (no problems were reported from '@typescript-eslint/no-namespace') [lint] [lint] ✖ 3 problems (0 errors, 3 warnings) [lint] 0 errors and 1 warning potentially fixable with the `--fix` option. [lint] [lint] npm run lint exited with code 0 > @fluidframework/[email protected] build:docs /workspaces/FluidFramework/common/lib/protocol-definitions > api-extractor run --local --typescript-compiler-folder ./node_modules/typescript && copyfiles -u 1 ./_api-extractor-temp/doc-models/* ../../../_api-extractor-temp/ api-extractor 7.22.2 - https://api-extractor.com/ Using configuration from ./api-extractor.json Analysis will use the bundled TypeScript version 4.5.5 API Extractor completed successfully @ghoshkaj ➜ .../FluidFramework/common/lib/protocol-definitions (depracate-ISummaryAuthor-ISummaryCommitter) $ ```

@tylerbutler Is there a specific rule you're thinking will fail? Just the tsdoc syntax mega-rule? My assumption would be that this would succeed, and the resulting bubbled-up documentation would just begin with "- ".

@Josmithr I thought tsdoc/syntax had one, but I think I got it confused with the one for params:

/** * The `@param` block should be followed by a parameter name and then a hyphen. */ ParamTagMissingHyphen = 'tsdoc-param-tag-missing-hyphen',

https://github.com/microsoft/tsdoc/blob/b0499dace630ca562728f18faf434eda1c782650/tsdoc/src/parser/TSDocMessageId.ts#L143-L146

I wish the spec was consistent and had dashes everywhere or nowhere.

You're right that it just includes the hyphen in the docs, which looks silly (but nothing bad other than that):

tylerbutler · 2022-07-01T05:59:55Z

yes, that is what I shared with her 🙂 - the problem is that she is facing problems building using codespaces - and I'm not familiar with it .

@ghoshkaj Let me know if I can help with codespaces!

Co-authored-by: Tyler Butler <[email protected]>

ghoshkaj · 2022-07-01T19:09:49Z

common/lib/protocol-definitions/src/summary.ts

@@ -7,13 +7,21 @@ export type SummaryObject = ISummaryTree | ISummaryBlob | ISummaryHandle | ISumm

 export type SummaryTree = ISummaryTree | ISummaryHandle;

+/**
+ * @deprecated ISummaryAuthor has never been used anywhere and is unlikely to be used in the


@tylerbutler should I also change the comment here to the generic "are deprecated and will be removed in a future release."?

Yeah, I would.

github-actions · 2022-07-01T20:48:05Z

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

deprecate ISummaryAuthor and ISummaryCommitter

531456e

ghoshkaj requested a review from a team as a code owner June 30, 2022 22:33

ghoshkaj self-assigned this Jun 30, 2022

github-actions bot added the breaking change This PR or issue would introduce a breaking change label Jun 30, 2022

ghoshkaj requested a review from NicholasCouri June 30, 2022 22:34

github-actions bot added the base: main PRs targeted against main branch label Jun 30, 2022

tylerbutler reviewed Jul 1, 2022

View reviewed changes

commit updated docs

117186c

ghoshkaj requested a review from a team as a code owner July 1, 2022 17:11

github-actions bot added the public api change Changes to a public API label Jul 1, 2022

ghoshkaj and others added 3 commits July 1, 2022 11:51

Update BREAKING.md

bc6a43e

Co-authored-by: Tyler Butler <[email protected]>

remove dashes

9838232

change BREAKING.MD comment

76ed757

ghoshkaj commented Jul 1, 2022

View reviewed changes

ghoshkaj added 2 commits July 1, 2022 19:10

pesky space

4b97726

modify comments

0518542

anthony-murphy approved these changes Jul 1, 2022

View reviewed changes

ghoshkaj merged commit 6d2d7f1 into microsoft:main Jul 1, 2022

ghoshkaj deleted the depracate-ISummaryAuthor-ISummaryCommitter branch July 1, 2022 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate ISummaryAuthor and ISummaryCommitter #10932

Deprecate ISummaryAuthor and ISummaryCommitter #10932

ghoshkaj commented Jun 30, 2022

ghoshkaj commented Jun 30, 2022

anthony-murphy commented Jul 1, 2022

NicholasCouri commented Jul 1, 2022 via email •

edited by ghost

Loading

tylerbutler left a comment

tylerbutler Jul 1, 2022

anthony-murphy Jul 1, 2022

ghoshkaj Jul 1, 2022

Josmithr Jul 1, 2022

tylerbutler Jul 1, 2022

tylerbutler commented Jul 1, 2022

ghoshkaj Jul 1, 2022

tylerbutler Jul 1, 2022

github-actions bot commented Jul 1, 2022

Deprecate ISummaryAuthor and ISummaryCommitter #10932

Deprecate ISummaryAuthor and ISummaryCommitter #10932

Conversation

ghoshkaj commented Jun 30, 2022

Description

Does this introduce a breaking change?

ghoshkaj commented Jun 30, 2022

anthony-murphy commented Jul 1, 2022

NicholasCouri commented Jul 1, 2022 via email • edited by ghost Loading

tylerbutler left a comment

Choose a reason for hiding this comment

tylerbutler Jul 1, 2022

Choose a reason for hiding this comment

anthony-murphy Jul 1, 2022

Choose a reason for hiding this comment

ghoshkaj Jul 1, 2022

Choose a reason for hiding this comment

Josmithr Jul 1, 2022

Choose a reason for hiding this comment

tylerbutler Jul 1, 2022

Choose a reason for hiding this comment

tylerbutler commented Jul 1, 2022

ghoshkaj Jul 1, 2022

Choose a reason for hiding this comment

tylerbutler Jul 1, 2022

Choose a reason for hiding this comment

github-actions bot commented Jul 1, 2022

NicholasCouri commented Jul 1, 2022 via email •

edited by ghost

Loading