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

refactor(api-markdown-documenter): Bundle Markdown and Html renderers into namespace exports #17576

Conversation

Josmithr
Copy link
Contributor

Reduces clutter in the API surface by scoping render functions.

New usage looks like the following:

import { MarkdownRenderer } from "@fluid-tools/api-markdown-documenter";
...
MarkdownRenderer.renderApiModel(...);

Breaking Change

This PR removes a number of free functions from the global namespace. Consumers will need to access via ApiItemUtilities.

@Josmithr Josmithr requested review from jenn-le, RishhiB and a team September 29, 2023 23:35
@Josmithr Josmithr requested review from a team as code owners September 29, 2023 23:35
@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Sep 29, 2023
Copy link
Contributor

@alexvy86 alexvy86 left a comment

Choose a reason for hiding this comment

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

The changes look fine. I have a bit of an issue with the structure of the readme though. There's a level 4 heading for MarkdownRenderer under Usage->Document Generation, then a level 3 one under Architectural Overview, and no corresponding ones for HtmlRendering... but there's a level 2 heading for HtmlRenderer. Seems like we could restructure it a bit at some point.

@Josmithr
Copy link
Contributor Author

Josmithr commented Oct 2, 2023

The changes look fine. I have a bit of an issue with the structure of the readme though. There's a level 4 heading for MarkdownRenderer under Usage->Document Generation, then a level 3 one under Architectural Overview, and no corresponding ones for HtmlRendering... but there's a level 2 heading for HtmlRenderer. Seems like we could restructure it a bit at some point.

I've been trying to make improvements here when I find downtime. I'll make a point to go over the README next time I'm here.

Thanks for the review!

@Josmithr Josmithr merged commit af58ac2 into microsoft:main Oct 2, 2023
25 checks passed
@Josmithr Josmithr deleted the api-markdown-documenter/bundle-renderer-exports branch October 2, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants