-
Notifications
You must be signed in to change notification settings - Fork 828
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 typedoc docs #56
Add typedoc docs #56
Conversation
CONTRIBUTING.md
Outdated
@@ -51,3 +51,4 @@ The `opentelemetry-node` project is written in TypeScript. | |||
- `yarn bootstrap` or `npm run bootstrap` Bootstrap the packages in the current Lerna repo. Installs all of their dependencies and links any cross-dependencies. | |||
- `yarn test` or `npm test` tests code the same way that our CI will test it. | |||
- `yarn fix` or `npm run fix` lint (and maybe fix) any changes. | |||
- `yarn run docs` or `npm run docs` to generate API documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be yarn docs
since Yarn doesn't require the "run" subcommand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes, done.
package.json
Outdated
@@ -12,7 +12,8 @@ | |||
"bootstrap": "lerna bootstrap", | |||
"bump": "lerna publish", | |||
"codecov": "lerna run codecov", | |||
"check": "lerna run check" | |||
"check": "lerna run check", | |||
"docs": "typedoc --tsconfig ./packages/opentelemetry-types/tsconfig.json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels wrong that the root would use a TS config from one of the packages. Should there be a tsconfig.json
for the root specifically? Maybe packages could inherit from it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if the doc is expected to only exist for opentelemetry-types
, maybe it would make sense to move the command there and simply do lerna run docs
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, done.
], | ||
"typedocOptions": { | ||
"name": "OpenTelemetry Documentation", | ||
"out": "docs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There could be other documentation in the docs
folder such as markdown files. The output for Typedoc should probably be a subfolder or simply a completely different folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed out
to documentation folder. PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue still exists as documentation
is equivalent to docs
. There should never be both folders at the same time since this would be very confusing. Maybe something like docs/out
or build/docs
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
], | ||
"typedocOptions": { | ||
"name": "OpenTelemetry Documentation", | ||
"out": "../../documentation", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the documentation is generated in the package, it should output in the package as well, not at the root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO as we are running command (yarn docs
) at the root level, it makes sense to generate docs there (to avoid confusion). WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The root command is irrelevant when it defers to Lerna, since this means there could be a docs
command in multiple packages. If that was the case and they all used a folder at the root, it could potentially result in conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically, will just have a single folder for API documentation and most likely will not add docs
script in another package to create conflicts. Anyways, I have changed the path to package-specific location (docs/out
).
@rochdev all the comments are addressed, PTAL once you get time. Thanks |
- Rename docs to documentation - Add lerna run docs
* Add typedoc docs * oops, yarn run docs -> yarn docs * Fix review comments - Rename docs to documentation - Add lerna run docs * set out to docs/out * Change out path
fix(pg): remove unneeded if guards
This PR adds typedoc support for generating API documentation.
Example:
You can build the html docs from this branch as: