Skip to content

feat: improve typedoc documentation generation #987

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 4 commits into from
Oct 28, 2019

Conversation

SaltedCaramelCoffee
Copy link
Contributor

@SaltedCaramelCoffee SaltedCaramelCoffee commented Oct 15, 2019

Checklist
  • npm test passes (tip: npm run autofix can correct most style issues)
  • documentation is changed or added
  • Improved TypeDocs generation to hide interfaces, type aliases and modules from navigation and page
  • Improved TypeDocs generation to expose first level properties of function/constructor arguments

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2019

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Oct 16, 2019

Codecov Report

Merging #987 into master will increase coverage by 22.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #987       +/-   ##
=========================================
+ Coverage   77.77%   100%   +22.22%     
=========================================
  Files           1      1               
  Lines           9      9               
  Branches        2      2               
=========================================
+ Hits            7      9        +2     
+ Misses          2      0        -2
Impacted Files Coverage Δ
test/resources/auth_helper.js 100% <0%> (+22.22%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aab649d...e7b3d8d. Read the comment docs.

Copy link
Contributor

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

@ExiliumAeternum This looks really great. I only have one more change I'd like to make:

When a service is selected, the first section to come up after the index is called Type aliases and defines the callback type. Since we are moving towards callbacks being deprecated, I don't think this should be the first section. Can you move this to the bottom, or remove it?

Also, I'm noticing that the constructor doc defines the UserOptions type but doesn't provide a link to view the type itself. This isn't a huge deal, but do you know why this is? I know this type comes from the core, but we document the properties in the comments above the constructor so I feel like we should generate docs for it

@SaltedCaramelCoffee
Copy link
Contributor Author

@dpopp07 The TSDoc specification shows examples to document only the parameters, not its first level properties. The extra parameters we put in the comment is ignored by TypeDoc since they are not used by the constructor.

Like you mentioned, since the type is from the core and not part of the SDK, I don't think TypeDoc found the definition of it and therefore it does not have its own page, and no link to the page either.

I've added logic to exclude sections that we are already excluding in the table of content. Please review the PR again.

Copy link
Contributor

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@SaltedCaramelCoffee SaltedCaramelCoffee merged commit 6bb1052 into master Oct 28, 2019
@SaltedCaramelCoffee SaltedCaramelCoffee deleted the feat-improve-typedoc branch October 28, 2019 15:45
@watson-github-bot
Copy link
Collaborator

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants