Skip to content

Auto-generated JS API docs & TS cleanup #2313

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

Merged
merged 19 commits into from
Mar 15, 2021

Conversation

letmaik
Copy link
Member

@letmaik letmaik commented Mar 12, 2021

Fixes #1765.

This uses sphinx-js and typedoc to generate API docs for the built-in parts of CCF. It does not (yet?) document the utility TypeScript module that can optionally be used for dealing with type conversions of KV map keys/values.

sphinx-js' TypeScript support is basic and the project acknowledges that and doesn't intend to work on it (since Mozilla doesn't use TypeScript themselves). Any improvements typically come from community PRs. I tried to work around various issues but it's not as pretty as it could be.

I also took the opportunity to split the built-in from the utility parts and move them into separate modules. There are no "automatic" breaking changes for existing apps since the original ccf.ts module has to be vendored.

screenshot

@letmaik letmaik requested a review from a team as a code owner March 12, 2021 15:47
@@ -34,6 +34,7 @@ jobs:
pip install wheel
pip install -U -r doc/requirements.txt
pip install -U -e ./python
npm install typescript [email protected]
Copy link
Member Author

Choose a reason for hiding this comment

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

sphinx-js doesn't support newer versions yet.

@@ -1,94 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably util is not the best name. Since these are all KV helpers, maybe something with kv in the name?

Copy link
Member

Choose a reason for hiding this comment

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

app.ts to match the C++ ccfapp namespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's a good name. We may have more modules in the future and collectively they may be in the "ccfapp" namespace but modules != namespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's ignore this for now and think about it again when we make it into a library.

@ghost
Copy link

ghost commented Mar 12, 2021

letmaik/js-api-docs@20568 aka 20210315.22 vs main ewma over 20 builds from 20172 to 20556
images

@letmaik letmaik marked this pull request as draft March 12, 2021 18:27
@letmaik letmaik marked this pull request as ready for review March 12, 2021 18:56
@achamayou
Copy link
Member

Two things that are unrelated to documentation but which this PR made obvious to me:

  1. We seem to have taken a rather random view to capitalising acronyms, so we have RsaOaep-like things and AESKW-like things. I don't particularly care which one we go with (although the latter makes more sense to me), but it's important to be consistent.
  2. All the crypto calls like generate key, wrap etc should be under ccf.crypto, and we should perhaps have ccf.conv for the conversion calls.

Copy link
Member

@achamayou achamayou left a comment

Choose a reason for hiding this comment

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

Thanks you for setting this up, I expect it will make the JS/TS interface considerably easier to use in practice.

@letmaik letmaik mentioned this pull request Mar 15, 2021
@letmaik
Copy link
Member Author

letmaik commented Mar 15, 2021

Two things that are unrelated to documentation but which this PR made obvious to me:

1. We seem to have taken a rather random view to capitalising acronyms, so we have RsaOaep-like things and AESKW-like things. I don't particularly care which one we go with (although the latter makes more sense to me), but it's important to be consistent.

2. All the crypto calls like generate key, wrap etc should be under `ccf.crypto`, and we should perhaps have `ccf.conv` for the conversion calls.

Yep, I opened #2315 for that.

@letmaik letmaik mentioned this pull request Mar 15, 2021
@ashamis ashamis merged commit beb4e43 into microsoft:main Mar 15, 2021
@letmaik letmaik deleted the letmaik/js-api-docs branch March 15, 2021 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update JS API docs
5 participants