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 support for markdown generation #3100

Merged
merged 2 commits into from
Jun 8, 2021
Merged

Add support for markdown generation #3100

merged 2 commits into from
Jun 8, 2021

Conversation

pmcollins
Copy link
Member

@pmcollins pmcollins commented May 4, 2021

This change contains a CLI tool, called 'docsgen', that generates markdown files for collector
components. The markdown files present the configuration metadata extracted by the configschema
API in a human readable form that can be used to manually configure the collector.

This change also makes some modifications to the package formerly knows as schemagen, renaming
it to configschema and exporting some things, because it no longer generates a schema yaml file, but
rather provides an equivalent API used by docsgen.

Also, this PR includes one sample, generated .md document: receiver/otlpreceiver/config.md

cmd/schemagen/docsgen/docsgen/templates/header.tmpl Outdated Show resolved Hide resolved
receiver/otlpreceiver/config.md Outdated Show resolved Hide resolved
receiver/otlpreceiver/config.md Show resolved Hide resolved
receiver/otlpreceiver/config.md Outdated Show resolved Hide resolved
receiver/otlpreceiver/config.md Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

This is great!

Can you please validate that this approach will work correctly for components imported from other repositories? For example we want OTLP receiver (which is in this repo) documentation in Collector contrib repo too.

@tigrannajaryan tigrannajaryan self-assigned this May 5, 2021
@pmcollins
Copy link
Member Author

This is great!

Can you please validate that this approach will work correctly for components imported from other repositories? For example we want OTLP receiver (which is in this repo) documentation in Collector contrib repo too.

Thanks Tigran. So I experimented a bit, and the docsgen API will work, with minor modifications, when called from a separate repo (e.g. contrib), for components that live in that repo. For components that live in a separate repo, we will need to be able to map instances of configuration types (e.g. otlpreceiver.Config) to the locations of their source code (to extract the comments), which appears to be doable. Can be done in a separate PR to keep this one smaller.

@tigrannajaryan
Copy link
Member

This is great!
Can you please validate that this approach will work correctly for components imported from other repositories? For example we want OTLP receiver (which is in this repo) documentation in Collector contrib repo too.

Thanks Tigran. So I experimented a bit, and the docsgen API will work, with minor modifications, when called from a separate repo (e.g. contrib), for components that live in that repo. For components that live in a separate repo, we will need to be able to map instances of configuration types (e.g. otlpreceiver.Config) to the locations of their source code (to extract the comments), which appears to be doable. Can be done in a separate PR to keep this one smaller.

Sounds good, let's finalize this PR and then improve it. We should aim to replace READMEs by this approach.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@tigrannajaryan
Copy link
Member

@pmcollins is this ready for review?

@pmcollins pmcollins marked this pull request as ready for review May 18, 2021 20:34
@pmcollins pmcollins requested a review from a team as a code owner May 18, 2021 20:34
@pmcollins
Copy link
Member Author

@pmcollins is this ready for review?

This is ready for review now. : )

@tigrannajaryan
Copy link
Member

This looks good to me. @bogdandrutu thoughts?

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

I like it overall, looked only at the result md and added some suggestions, I trust @tigrannajaryan that he reviewed the code that generates the md

receiver/otlpreceiver/config.md Show resolved Hide resolved
receiver/otlpreceiver/config.md Show resolved Hide resolved
@pmcollins
Copy link
Member Author

Thanks for reviewing @tigrannajaryan and @bogdandrutu 🙇
Any remaining questions/comments/concerns?

@tigrannajaryan
Copy link
Member

LGTM. Please rebase from main.

@pmcollins
Copy link
Member Author

LGTM. Please rebase from main.

Sure, done.

@tigrannajaryan tigrannajaryan merged commit 40f7db7 into open-telemetry:main Jun 8, 2021
@tigrannajaryan
Copy link
Member

Thanks @pmcollins !

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.

3 participants