-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: support overview page #365
Conversation
docfx_yaml/markdown_utils.py
Outdated
@@ -280,7 +315,13 @@ def move_markdown_pages( | |||
|
|||
"reference.md", # Reference docs overlap with Overview. Will try and incorporate this in later. | |||
# See https://github.com/googleapis/sphinx-docfx-yaml/issues/106. | |||
"summary_overview.md", # Already included in the TOC. | |||
"overview_content.md", # Will be included in the summary_overview page. |
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.
Nit: included in
-> content will be merged into summary_overview page.
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.
Done.
"overview_content.md", # Will be included in the summary_overview page. | ||
] | ||
|
||
# Use this to move the page but do not add them in the TOC. |
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.
Should this still be moved to be included in the reference docs after it is merged? If it's only intended to be dropped from the TOC can you just rm
the file after it's been merged?
If so, I think you can remove the ignore_in_toc
logic altogether.
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.
Ah, this is to (1) move the file from docs/summary_overview.md
to output_location/summary_overview.md
.
In the process we also (2) include it in app.env.moved_markdown_pages
and (3) include it in the TOC as well.
We do need the summary_overview page in the output location for it to be used. However we don't need it in (2) or (3) because triggering either conditions will either (a) remove the page, or (b) include it again in the TOC.
tests/test_markdown.py
Outdated
# The content should be the same as base, and not throw any errors. | ||
"tests/markdown_example_base.md", | ||
"tests/markdown_example_does_not_exist.md", | ||
"tests/markdown_example_dne_expected.md", |
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.
Nit: I think it's a bit clearer if you leave the expected result as tests/markdown_example_base.md
because it makes it's clear it's the same file (so it has the same contents).
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.
Done!
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.
Thanks! No logic changes.
docfx_yaml/markdown_utils.py
Outdated
@@ -280,7 +315,13 @@ def move_markdown_pages( | |||
|
|||
"reference.md", # Reference docs overlap with Overview. Will try and incorporate this in later. | |||
# See https://github.com/googleapis/sphinx-docfx-yaml/issues/106. | |||
"summary_overview.md", # Already included in the TOC. | |||
"overview_content.md", # Will be included in the summary_overview page. |
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.
Done.
"overview_content.md", # Will be included in the summary_overview page. | ||
] | ||
|
||
# Use this to move the page but do not add them in the TOC. |
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.
Ah, this is to (1) move the file from docs/summary_overview.md
to output_location/summary_overview.md
.
In the process we also (2) include it in app.env.moved_markdown_pages
and (3) include it in the TOC as well.
We do need the summary_overview page in the output location for it to be used. However we don't need it in (2) or (3) because triggering either conditions will either (a) remove the page, or (b) include it again in the TOC.
tests/test_markdown.py
Outdated
# The content should be the same as base, and not throw any errors. | ||
"tests/markdown_example_base.md", | ||
"tests/markdown_example_does_not_exist.md", | ||
"tests/markdown_example_dne_expected.md", |
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.
Done!
…docfx-yaml into support_overview_page
Adds support for
summary_overview.md
page. Gives users an option to also haveoverview_content.md
file in their libraries, and if found, that content will be prepended to the existing Overview page content.summary_overview.md
page will be generated separately from the tool, through Synthtool. Same foroverview_content.md
file which will be supplied by library maintainers.Adjusted a bit of the
move_markdown_pages
spacing.Towards b/331981881.