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

doc: Validate documentation rst with phpdocumentor #2646

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Jun 19, 2024

Q A
Type improvement
BC Break no
Fixed issues #2640

Summary

Use phpdocumentator cli to validate rst files.

@GromNaN GromNaN requested a review from jmikola June 19, 2024 11:11
@GromNaN GromNaN changed the title Validate documentation rst with phpdocumentor doc: Validate documentation rst with phpdocumentor Jun 19, 2024
@@ -1,7 +1,7 @@
Implementing ArrayAccess for Domain Objects
===========================================

.. sectionauthor:: Roman Borschel ([email protected])
.. sectionauthor:: Roman Borschel <[email protected]>
Copy link
Member Author

Choose a reason for hiding this comment

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

WARNING: Content of .. sectionauthor:: name <email> must specify a name and can also specify an email

@@ -1,3 +1,4 @@
:orphan:
Copy link
Member Author

Choose a reason for hiding this comment

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

WARNING: Document "sidebar" isn't included in any toctree. Include it in a .. toctree:: directive or add :orphan: in the first line of the rst document {"rst-file":"sidebar.rst"} []

Copy link
Member

Choose a reason for hiding this comment

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

I noted that the ORM's documentation workflow manually added :orphan: before validating. Is there a particular reason that approach was preferred there instead of modifying the RST directly as you've done here?

I don't have any objections to this, but I wonder if it'd be worth suggesting upstream to the ORM team.

Copy link
Member

Choose a reason for hiding this comment

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

orphan is not supported by doctrine/rst-parser, which we still use for generating our docs

This means that if you don't use the trick I used, you are going to end up with :orphan: being printed above the sidebar of your docs 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the clarification, I've adopted your solution that adds :orphan: in the job.

Comment on lines +98 to +99
.. |FQCN| raw:: html
<abbr title="Fully-Qualified Class Name">FQCN</abbr>
Copy link
Member Author

Choose a reason for hiding this comment

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

WARNING: No replacement was found for variable |FQCN| {"rst-file":"reference/custom-mapping-types.rst"}

@@ -1,3 +1,4 @@
:orphan:
Copy link
Member

Choose a reason for hiding this comment

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

I noted that the ORM's documentation workflow manually added :orphan: before validating. Is there a particular reason that approach was preferred there instead of modifying the RST directly as you've done here?

I don't have any objections to this, but I wonder if it'd be worth suggesting upstream to the ORM team.

@@ -0,0 +1,39 @@
name: "Documentation"
Copy link
Member

Choose a reason for hiding this comment

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

Noted that this was adapated from the ORM action.

@GromNaN GromNaN merged commit 517fb9a into doctrine:2.9.x Jun 20, 2024
17 of 18 checks passed
@GromNaN GromNaN deleted the doc-code-validator branch June 20, 2024 10:33
@GromNaN GromNaN added the CI label Jul 5, 2024
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.

5 participants