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 :showdoc: directive (RFC #11) #15294

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

nobodywasishere
Copy link
Contributor

@nobodywasishere nobodywasishere commented Dec 19, 2024

Implements RFC#0011.

Starting as a draft for now to get feedback.

@nobodywasishere
Copy link
Contributor Author

(apologies for force push, force of habit)

@nobodywasishere nobodywasishere marked this pull request as ready for review December 20, 2024 02:04
@ysbaddaden
Copy link
Contributor

That looks pretty good and seems to do the job well 👍

@Blacksmoke16
Copy link
Member

Given this PR is implementing an RFC for :showdoc: should we save the extra stuff for future PRs? There seems to be more going on than just supporting this new directive.

@nobodywasishere
Copy link
Contributor Author

There seems to be more going on than just supporting this new directive.

What stuff are you referring to?

@Blacksmoke16
Copy link
Member

From what I can tell this is also enabling doc support on lib related types yea?

@nobodywasishere
Copy link
Contributor Author

Yes that's true. I'm fine splitting it into two PRs, but enabling documentation support for lib types was the original purpose for the :showdoc: directive. The RFC encapsulates both the documentation of lib types, and the directive for enabling the documentation of those types (alongside private/protected ones).

@straight-shoota
Copy link
Member

I think splitting this into separate PRs is a good idea. The changes are all working towards the goal to support API docs for lib bindings. But they're not directly related. The doc generator changes and the parser changes for example do not strictly depend on each other. We can merge either one individually and have a subset of features available.
Splitting makes the changes smaller, easier to review, and easier to reason about in the future when someone is wondering what's been happening here.

@nobodywasishere
Copy link
Contributor Author

Sounds good. How should I go about splitting this PR? As we discourage force-pushing, my thought is closing this PR and opening two others (one for lib docs, another for showdoc directive)

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

Successfully merging this pull request may close these issues.

4 participants