-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
Directive refactoring #698
Directive refactoring #698
Conversation
I will try to handle this PR tomorrow, it's too large for the current late-night round. |
b06df5b
to
9e69659
Compare
Note, both of these PRs are on top of the changes introduced by jakobandersen#698. Links to PRs for future reference: - breathe-doc#698 - breathe-doc#711 - breathe-doc#723 Signed-off-by: Abrar Rahman Protyasha <[email protected]>
Note, both of these PRs are on top of the changes introduced by upstream PR breathe-doc#698. Links to PRs for future reference: - breathe-doc#698 - breathe-doc#711 - breathe-doc#723 Signed-off-by: Abrar Rahman Protyasha <[email protected]>
9e69659
to
aa97f6f
Compare
This should be ready for review now. It's quite a lot of changes, so reviewing each commit in turn may be the easiest. |
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.
Looks good to me and the output is the same.
I'm not used to leading underscores on classes but I can understand the motivation.
Merge when you're happy @jakobandersen :) |
Thanks |
🎉 |
This should not change any behaviour (tested with Breathes own documentation).
Merge
directive/
anddirectives.py
into adirectives/
module with the directives in separate files based on their types.Note, the union directive is currently an "item" directive but should probably be changed to be a
class-like
directive so options like:members:
become available. However, there is some strange namespacing rendering differences that needs to be ironed out first.