[MMB-156] Add documentation comments and generate HTML documentation #204

lawrence-forooghian · 2023-10-04T13:23:42Z

What's done

This PR:

takes the Spaces documentation on ably.com (including the fixes from Further fixes to Spaces docs docs#2015) and the documentation in the repo’s docs/class-definitions.md and adds them to the project as TSDoc documentation comments;
does some basic editing on these comments;
uses TypeDoc to generate HTML documentation based on these comments, and uploads this HTML documentation to sdk.ably.com
- this is based on the approach used in ably-js
- you can view the generated HTML documentation using the "View deployment" button on this PR

See commit messages for more details.

What's left to do

I think there is still plenty of editing that can be done to the documentation that this PR adds, to make it look less like it was just copied and pasted from somewhere else. This is not work that I particularly excel at, especially given my relative unfamiliarity with the SDK still. I think it would be more effectively and efficiently done by someone like @m-hulbert. Whether we want to merge this PR and do further editing at a later date, or hold off on merging this PR until someone can edit it, I'll leave to @dpiatek and @Srushtika to decide.

dpiatek · 2023-10-16T09:48:46Z

@lawrence-forooghian is it possible to deploy to https://sdk.ably.com/ the docs so we can inspect them before we merge this PR?

dpiatek · 2023-10-05T16:11:46Z

package.json

@@ -55,7 +56,8 @@
    "vitest": "^0.34.3"
  },
  "dependencies": {
-    "nanoid": "^4.0.2"
+    "nanoid": "^4.0.2",
+    "typedoc": "^0.25.1"


typedoc would be a devDependency, we don't need it at runtime

Oops, thanks — I've fixed that.

dpiatek · 2023-10-16T09:43:08Z

docs/typedoc/intro.md

+>
+> This documentation is a work in progress and will be improved over time.
+
+For alternative documentation, see our [documentation website](https://ably.com/docs/spaces).


Is this file necessary (i.e. is included in the generated output on the remote)?

Yeah, take a look at the generated HTML documentation and you'll see that it's the start page for those docs.

(See the readme configuration in typedoc.json. If you don't specify one, then it'll use the repo’s README, which I thought looked a bit odd. We do the same thing — custom start page for generated docs — in ably-js, too.)

I see, thanks. Can we please change the contents to the following:

# Ably Spaces SDK This is the API reference for Ably’s Spaces SDK, generated with [typedoc](https://typedoc.org/). To find more about Spaces and how to get started head to [Ably documentation website](https://ably.com/docs/spaces).

OK, have done (with some tweaks to the wording).

dpiatek · 2023-10-16T09:50:55Z

src/Cursors.ts

+/**
+ * > **Documentation source**
+ * >
+ * > The following documentation is copied from the [Spaces documentation website](https://ably.com/docs/spaces).


I'm somewhat uneasy about these big chunks of documentation being copied from docs. These are bound to get very quickly out of sync (in fact - they already are as we now have a textile and markdown version of the same docs). Is this how it is done in other SDKs or a normal practice that I'm unaware of? Maybe we could have a summary here and link to /docs/spaces?

@m-hulbert any thoughts on this?

For the Ably client SDKs, the tech writers have written dedicated documentation comments. See https://github.com/ably/sdk-api-reference/blob/main/descriptions.md. The Ably client SDKs (e.g. ably-js, ably-cocoa) have then copied and pasted these descriptions and made some platform-specific edits.

Since we don't have the same thing in Spaces (high-quality docstrings written by tech writers) I instead tried to make use of whichever tech writer-written documentation we did have, hence the copying and pasting from the docs repo. (This documentation was in general of a higher quality that that in class-definitions.md.) I do think that if we had time then it would be good to do the same thing as our other SDKs — that is, ask Mark to write proper docstrings for us to use.

As for whether — if we decide to make use of the content in the docs repo — we should be linking to it or copying-and-pasting it, I don't know. I thought we wanted these documentation comments to be useful by themselves, without needing a user to navigate to a separate website. But it's probably a question for @Srushtika.

I see, thanks for explaining @lawrence-forooghian. It sounds this falls under what we discussed in our stand-ups, and this is the part that @m-hulbert and the team can update over time. I was thrown off by the comment The following documentation is copied from the [...] as it suggested to me that there is supposed to be a sync of some kind, but I can see that's not the intent here (or rather the decision is going to be with the docs team).

however, I think the comment, which I can now see is visible on https://sdk.ably.com/builds/ably/spaces/pull/204/typedoc/ is I think unnecessary. The fact the docs are copied, at this point in time, is I think irrelevant to the user and is more for our information. Could we remove them please

I did that because I wanted it to be clear to users that the documentation hadn't been properly edited yet and might not make full sense in this new context. Also, there are places where we have both website documentation and class-definitions.md documentation and it doesn't read well if you just put one after another without anything to separate them.

The correct solution is to edit the documentation so that none of this stuff is needed, but I think that in the meantime it makes sense to keep these things there to make it clear that we know that the documentation might not be great. I am happy to remove all of these markers though if you prefer. What do you think?

(I do agree that the text "The following documentation was copied from the former docs/class-definitions.md document." is quite nonsensical though — why would a user care about a document that no longer exists...)

I'd for removing them - we're not planning to cut a release yet so there should be plenty of time for the docs team to review.

OK, cool, will do.

lawrence-forooghian · 2023-10-16T11:51:17Z

@lawrence-forooghian is it possible to deploy to https://sdk.ably.com/ the docs so we can inspect them before we merge this PR?

This is already being done; just click the "View deployment" button on the PR (it links to https://sdk.ably.com/builds/ably/spaces/pull/204/typedoc/).

dpiatek

Copied all relevant-seeming documentation from website (at commit cb5de6a) to roughly appropriate places in code. Done: - avatar.textile - cursors.textile - locations.textile - locking.textile - space.textile Skipped: - setup.textile - index.textile

Copied all documentation from that document to appropriate places in code.

Remove all of the stuff that is (or aims to be) a declaration that already exists in the source code.

In an upcoming commit, I’m going to use Pandoc to convert the website documentation from Textile to Markdown. However, Pandoc’s parser (incorrectly, I think) thinks that the <p> tag belongs to the link’s URL, so split it up.

We’ll use this very shortly.

Ran `npx ts-node scripts/convert_website_documentation.ts && npm run format`.

This reverts 17a693d; the script is no longer needed after we ran it in e7f8ca4.

The docs repo has a bunch of CSS for making these stand out visually, but we don’t have any of that when using TypeDoc (I guess I could configure it but don’t really want to put time into it now), so instead just use the tools at our disposal to make these sections stand out visually.

The relative links were designed for content contained within the ably.com/docs site, which the HTML documentation we generate with TypeDoc will not necessarily be.

I think it reads a bit better, at least in this context.

I had a quick scan of the docstrings added in e1b9359 and, where I noticed documentation that seemed like it would be more appropriate against another member, I moved it there and added a note explaining that it had been moved (so that the original content would continue to read OK). It’s not comprehensive and I think that a deeper reading of the documentation content would identify more that can be done here.

This is based on my pretty limited knowledge of the SDK and some scanning of the documentation. And I didn’t put a great amount of time into it. There’s plenty of room for improvement.

This is largely copied from the way we do things in ably-js (as of its commit 76a5475). I’ve made a couple of tweaks to the config, though: - Added all types (except Project, which I have no idea how to document, and TypeLiteral, which would require us to split out every type literal into its own type, which we don’t want) to requiredToBeDocumented. I took the list from [1]. - Excluded internal and private stuff from documentation (see ccafd10). [1] https://github.com/TypeStrong/typedoc/blob/f2d2abe054feca91b89c00c33e1d726bbda85dcb/src/lib/models/reflections/kind.ts#L7

All of the valuable content from document was copied to TSDoc comments in 95ed746, and I don’t want us to need to maintain two copies of this content. So, replace links to this document with links to the TypeDoc-generated documentation on sdk.ably.com (it’s not ideal that these links always link to the documentation for the `main` branch, regardless of which version of the codebase the user is browsing, but we can think about how to fix that later) and delete it.

lawrence-forooghian requested review from dpiatek, m-hulbert and Srushtika and removed request for dpiatek October 4, 2023 13:23

github-actions bot temporarily deployed to staging/pull/204/typedoc October 4, 2023 13:24 Inactive

lawrence-forooghian marked this pull request as ready for review October 4, 2023 13:28

lawrence-forooghian force-pushed the prepare-for-documentation-comments branch from 1234271 to 0dc5de5 Compare October 9, 2023 13:13

dpiatek force-pushed the prepare-for-documentation-comments branch from 0dc5de5 to 20ef7aa Compare October 16, 2023 09:11

dpiatek suggested changes Oct 16, 2023

View reviewed changes

lawrence-forooghian force-pushed the prepare-for-documentation-comments branch from 20ef7aa to f053f80 Compare October 16, 2023 11:53

Base automatically changed from prepare-for-documentation-comments to main October 16, 2023 11:56

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from ccb6658 to e2d434b Compare October 16, 2023 12:01

github-actions bot temporarily deployed to staging/pull/204/typedoc October 16, 2023 12:02 Inactive

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from e2d434b to eaede99 Compare October 16, 2023 12:09

github-actions bot temporarily deployed to staging/pull/204/typedoc October 16, 2023 12:10 Inactive

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from eaede99 to 665c6d9 Compare October 16, 2023 18:17

github-actions bot temporarily deployed to staging/pull/204/typedoc October 16, 2023 18:18 Inactive

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from 665c6d9 to d0365b2 Compare October 17, 2023 11:44

github-actions bot temporarily deployed to staging/pull/204/typedoc October 17, 2023 11:45 Inactive

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from d0365b2 to 796f8c8 Compare October 17, 2023 12:24

github-actions bot temporarily deployed to staging/pull/204/typedoc October 17, 2023 12:25 Inactive

dpiatek approved these changes Oct 17, 2023

View reviewed changes

lawrence-forooghian added 5 commits October 17, 2023 09:29

Add class-definitions.md documentation as TSDoc comments

95ed746

Copied all documentation from that document to appropriate places in code.

Remove redundant documentation added in 95ed746

a77ab99

Remove all of the stuff that is (or aims to be) a declaration that already exists in the source code.

Split </p> from end of link

2a3d732

In an upcoming commit, I’m going to use Pandoc to convert the website documentation from Textile to Markdown. However, Pandoc’s parser (incorrectly, I think) thinks that the <p> tag belongs to the link’s URL, so split it up.

Add a script to convert website documentation from Textile to Markdown

17a693d

We’ll use this very shortly.

lawrence-forooghian added 10 commits October 17, 2023 09:29

Convert website documentation from Textile to Markdown

e7f8ca4

Ran `npx ts-node scripts/convert_website_documentation.ts && npm run format`.

Remove scripts/convert_website_documentation.ts

efe4697

This reverts 17a693d; the script is no longer needed after we ran it in e7f8ca4.

Make links to external documentation absolute

a5ee043

The relative links were designed for content contained within the ably.com/docs site, which the HTML documentation we generate with TypeDoc will not necessarily be.

Convert documentation links into TSDoc @link

37ec1e2

Tweak references to "namespace" in docstrings added in e1b9359

46fae57

I think it reads a bit better, at least in this context.

Write rudimentary documentation where none currently exists

a2a49fb

This is based on my pretty limited knowledge of the SDK and some scanning of the documentation. And I didn’t put a great amount of time into it. There’s plenty of room for improvement.

lawrence-forooghian force-pushed the MMB-156-add-documentation-comments branch from 796f8c8 to 2c63012 Compare October 17, 2023 12:30

github-actions bot deployed to staging/pull/204/typedoc October 17, 2023 12:30 View deployment

lawrence-forooghian merged commit c58418d into main Oct 17, 2023
6 checks passed

lawrence-forooghian deleted the MMB-156-add-documentation-comments branch October 17, 2023 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MMB-156] Add documentation comments and generate HTML documentation #204

[MMB-156] Add documentation comments and generate HTML documentation #204

lawrence-forooghian commented Oct 4, 2023 •

edited

Loading

dpiatek commented Oct 16, 2023

dpiatek Oct 5, 2023

lawrence-forooghian Oct 16, 2023

dpiatek Oct 16, 2023

lawrence-forooghian Oct 16, 2023 •

edited

Loading

dpiatek Oct 16, 2023

lawrence-forooghian Oct 17, 2023

dpiatek Oct 16, 2023

lawrence-forooghian Oct 16, 2023

dpiatek Oct 16, 2023

dpiatek Oct 16, 2023

lawrence-forooghian Oct 17, 2023

lawrence-forooghian Oct 17, 2023

dpiatek Oct 17, 2023

lawrence-forooghian Oct 17, 2023

lawrence-forooghian Oct 17, 2023

lawrence-forooghian commented Oct 16, 2023

dpiatek left a comment

[MMB-156] Add documentation comments and generate HTML documentation #204

[MMB-156] Add documentation comments and generate HTML documentation #204

Conversation

lawrence-forooghian commented Oct 4, 2023 • edited Loading

What's done

What's left to do

dpiatek commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lawrence-forooghian Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lawrence-forooghian commented Oct 16, 2023

dpiatek left a comment

Choose a reason for hiding this comment

lawrence-forooghian commented Oct 4, 2023 •

edited

Loading

lawrence-forooghian Oct 16, 2023 •

edited

Loading