Skip to content
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

Add API doc generation #12

Merged
merged 12 commits into from
Sep 4, 2018
Merged

Add API doc generation #12

merged 12 commits into from
Sep 4, 2018

Conversation

btlewan
Copy link
Contributor

@btlewan btlewan commented Aug 27, 2018

Uses infrastructure provided in the Interbit plugin to:

  • Fetch the Interbit repo during doc generation.
  • Use jsdoc to parse source comments from select Interbit packages.
  • Convert jsdoc's JSON output into Asciidoc markup.
  • Updated reference documentation that includes the generated Asciidoc markup.

This strategy should allow use to enhance documentation with examples and other relevant content without cluttering the source code with such content.

Run ./make.sh from the repo root to generate the documentation + API content.

The doc CI compares generated Asciidoc markup for the API with included files; warnings are displayed if any files to be included do not exists and if any generated files are not included.

Run npm run test to execute all of the doc CI checks.
Run npm run includes to just run the include checks.
Run npm run spell to just check the spelling within the docs source.

* Add a jsdoc configuration file.
* Runs jsdoc on specific packages, capturing API details from source
  comments.
* Converts the JSON from jsdoc into Asciidoc markup.
* Adds reference documentation that includes the generated Asciidoc
  markup.
* Adds NPM commands to package.json that allow the spell and includes CI
  checks to be executed explicitly.
* make.sh now uses console colors to delineate processing stages.
@btlewan btlewan temporarily deployed to interbit-docs-dev-pr-12 August 27, 2018 23:35 Inactive
@btlewan btlewan temporarily deployed to interbit-docs-dev-pr-12 August 27, 2018 23:35 Inactive
SUMMARY.md Outdated
@@ -44,6 +44,7 @@
* [Constants](reference/interbit/constants.md)
* [configSelectors](reference/interbit/configSelectors.md)
* [createChains()](reference/interbit/createChains.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Per your question in standup, it looks like we have an error in this API documentation. createChains is not actually a function, but is an object containing two functions: createChainsFromManifest and createChainsFromConfig(deprecated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we shake the API tree, issues can sometimes fall out. 😄 I'll file an issue for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@@ -57,7 +58,10 @@
* [manifest file](reference/interbit-cli/manifest.adoc)
* [start](reference/interbit-cli/start.md)
* [interbit-covenant-tools](reference/interbit-covenant-tools/README.md)
* [validate()](reference/interbit-covenant-tools/validate.adoc)
Copy link
Contributor

Choose a reason for hiding this comment

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

These function names are not clear when they do not contain the additional scope of the exported object tree. Is there some way we could include the property path from module exports in the documentation for clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may be reading the source incorrectly, but it looks to me like the top-level exports for interbit-covenant-tools includes validate(). So the documentation hierarchy matches exactly. Are you looking for some sort of breadcrumb, so that when you're viewing validate() you can see that it's a top-level function?

As an aside, the validate() function's source comments are lacking an @returns, and may need review for accuracy.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct, I had thought this was one of validateManifest or validateConfig but it is not. I forgot that export was in interbit-covenant-tools. Thanks!

@@ -1,34 +1,41 @@
#!/bin/bash

echo 'Building documentation, please wait... '
echo -e "\033[34mBuilding documentation, please wait...\033[0m"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 colours!

@@ -23,6 +24,8 @@
"start": "node index.js",
"build": "bash make.sh",
"test": "node node_modules/gitbook-plugin-interbit/scripts/doc_checks.js",
"spell": "node node_modules/gitbook-plugin-interbit/scripts/doc_checks.js -c spelling",
Copy link
Contributor

Choose a reason for hiding this comment

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

🔮 🌔

@nicholeous
Copy link
Contributor

Looks good! I just have a question about indicating the path for the export that maybe doesn't need to be addressed immediately but perhaps end up as a discussion & ticket instead.

@btlewan btlewan temporarily deployed to interbit-docs-dev-pr-12 August 31, 2018 00:50 Inactive
@btlewan btlewan temporarily deployed to interbit-docs-dev-pr-12 August 31, 2018 16:50 Inactive
Copy link
Contributor

@nicholeous nicholeous left a comment

Choose a reason for hiding this comment

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

Looking good, a couple of minor changes

SUMMARY.md Outdated
* [connectToPeers()](reference/interbit/connectToPeers.adoc)
* [create()](reference/interbit/create.adoc)
* [createChains](reference/interbit/createChains.adoc)
* [createChainsFromConfig()](reference/interbit/createChainsFromConfig.adoc)
Copy link
Contributor

Choose a reason for hiding this comment

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

createChainsFromConfig() and createChainsFromManifest() are a subset of createChains

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, they are. What's the purpose behind that structure? It requires all callers to know about the structuring, but I can't tell why that hoop needs to be jumped through.

I'll update the TOC accordingly.

Interbit offers the same features as the `interbit-cli` but in small
functions requireable for programmatically managing your chains.

The `interbit` package contains function to help you to:
Copy link
Contributor

Choose a reason for hiding this comment

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

function functions

@@ -0,0 +1,3 @@
const { createChains: { createChainsFromManifest } } = require('interbit')

const { ??? } = await createChainsFromManifest(cli, manifest)
Copy link
Contributor

Choose a reason for hiding this comment

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

The promise this function returns resolves to nothing. The context from createChainsFromConfig is generated dynamically during the function operation whereas when calling createChainsFromManifest it is already present in the manifest passed into params so unnecessary to return.

Copy link
Contributor

@nicholeous nicholeous left a comment

Choose a reason for hiding this comment

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

😸

@btlewan btlewan merged commit d985d2d into master Sep 4, 2018
@btlewan btlewan deleted the ewan/add-api-parsing branch September 4, 2018 18:01
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.

None yet

2 participants