-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
Improve php-src docs sphinx build, also on *nix #16743
Conversation
7319d98
to
3240524
Compare
Thank you! This looks generally good to me (although I have very limited knowledge about Sphinx), but I'm slightly concerned that the build scripts for Windows and POSIX will diverge in the future. Unfortunately, there is not much common ground; maybe we should add some PHP to the mix? Another option would be to drop the Windows build script altogether; users could then use MSYS2 or Cygwin. That might be a hurdle for some, though. |
Yes, that is something to have under watch, always, but here I see little danger of that, as the build is driven by Python and Docutils (Sphinx) and is basically a simple command. I have spotted no additional magic in the configuration.
No, not from my perspective. Preferable scripts should be written in Pyhton for a Docutils based build (Spinx).
With these requested changes the build instructions should work for users on both *nix and win systems, the overhead for that so far (one small batch and one small makefile) should not really bug us. And if it does I can offer to build back. Build instructions remain streamlined:
and that's the most important for maintainability from my end. |
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.
Hi. Thanks for working on improving the docs.
Related: #16676
This can target master, we're not planning to maintain different versions of the document. Differences between versions can be referenced within the document itself.
Another option would be to drop the Windows build script altogether
I agree with this. Note that I did not add this bat file, it ships by default in the sphinx template. If you're developing on Windows, wsl and cygwin seem like good options.
Yeah, then please drop make.bat. |
3240524
to
bd09b5c
Compare
Rebased.
Dropped. |
1a3bf7c
to
d42b8c0
Compare
Updated. GH CI docs workflow now uses Another late question of mine was to change .rst file line length to 80 characters, previously 100. This is aligned with both .md files (the other documentation file format next to plain text) and with .rst files in php/policies (ref: php/policies@9a97c2f) which uses the same tooling as Streamling here is beneficial I think, but unfortunately I'm not aware of the rationale behind a line length of 100. What I could gather from "upstream" (reStructuredText markup in the Python Developer's Guide), 80 is also mentioned and the rstfmt utility shows the additional behavior when I build with it:
It is a trivial change and I previewed it locally. Building the HTML docs yielded the same objects (apart from some woff/2 ones that can be ignored in the test comparison). Please let me know any objections. |
Thanks for fixing the CI MAC workflows @cmb69 ! |
965343c
to
9d2556b
Compare
I don't see a good argument for switching to |
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.
As mentioned, please revert to 100 chars per line. I don't see any good arguments for changing this. The rest looks reasonable. Thanks for working on it.
So what is the argument for 100 chars per line in the original rstfmt formatting suggestion? I could find no further reference to it in all projects I'm personally aware of. Where does it originate from? What is its rationale? @iluuu1994 |
9d2556b
to
1c228c0
Compare
rebased on target and removed the commit changing the line-length. |
In 19d2b84 ("Create book for docs", 2024-01-30) the build of the php-src documentation has been introduced. It is based on reStructuredText (rst) [Docutils] for its source files, this stems from the sphinx-build utility in use to build the static HTML pages of the php-src documentation. The maximum line length of these text files has been set to 100 characters in 19d2b84 ("Create book for docs", 2024-01-30), the rationale is unknown to the documenting author at time of writing this message. This formatting constraint is applied with the rstfmt utility [rstfmt] via its invocation (documented in CI build instructions and README.md:) rstfmt -w 100 source The `-w, --width` option takes a WIDTH argument that is "the target line length in characters" (cf. `rstfmt --help`.) There is also an `--ext EXT` argument option, that is "the extension of files to look at when passed a directory" ("source" is the name of a directory in the invocation above) and defaults to "rst". Henceforth, the editor configuration [EditorConfig] can benefit from documenting this expectation in the repositories .editorconfig file, which has been introduced already earlier in 5c38fbe ("Added editorconfig file", 2016-06-26). [Docutils]: https://docutils.sourceforge.io/index.html "Docutils: Documentation Utilities — Written in Python, for General- and Special-Purpose Use" [rstfmt]: https://github.com/dzhu/rstfmt "A formatter for reStructuredText" [EditorConfig]: https://editorconfig.org/ "EditorConfig helps maintain consistent coding styles for multiple developers working on the same project across various editors and IDEs"
In 19d2b84 ("Create book for docs", 2024-01-30) the php-src documentation (php-src docs) build has been introduced, yet the build instructions, namely `make html`, did not yield the expected results within the parenting setup of the php-src project on *nix systems. The reason is that the `make html` build instruction does not execute the make.bat file which contains the recipe to build the static HTML pages. It is an unused leftover file from initializing the project with sphinx-quickstart. [1] Removing it in and adding a Makefile suffices to recover the build of php-src ./docs on a *nix system. Formatting constraints checked in the docs workflow in CI update use the make file to make sure the commands stay consistent and the build is managed by the build manager. [1]: https://www.sphinx-doc.org/en/master/man/sphinx-quickstart.html "sphinx-quickstart is an interactive tool that asks some questions about your project and then generates a complete documentation directory and sample Makefile to be used with sphinx-build(1)."
Define the required packages to install for the php-src docs build in the docs/requirements.txt file: 1) Sphinx 2) sphinx-design 3) sphinxawesome-theme 4) rstfmt This should also later on ease the use of a requirements_frozen.txt file to pin the build dependencies if needed/wanted. Additionally, some formatting corrections in README.md (based on the profile in .editorconfig) as well as adding the recommendation to use a Python virtual environment. Python3 and Pip were already named, and with Python3 there is the venv module (Python 3.3; Sep 2012) to manage these so-called python virtual environments [venv], which are commonly a preferred way to install dependencies within development projects and build systems. [venv]: https://docs.python.org/3/library/venv.html "venv — Creation of virtual environments — Python documentation"
For the configured Awesome Sphinx Theme [1] highlighting extension, the sphinx-build currently yields the following diagnostics: WARNING: while setting up extension sphinxawesome_theme.highlighting: \ You no longer have to include the `sphinxawsome_theme.highlighting` \ extension. This extension will be removed in the next major release. (via `make html`, the configuration file is `source/conf.py`.) The diagnostic message was introduced by sphinxawesome-theme 5.2.0, released May 31, 2024. [2], [3] Removing the extension from the list of extensions in the configuration file levitates. No changes to requirements.txt, the extension was transitive as bundled by the Awesome Sphinx Theme [1], and 5.2.0 deprecates it with the new feature to "Support `pygments_style_dark` option that allows you to set a different syntax highlighting scheme in light and dark modes." [3] [1]: https://sphinxawesome.xyz/ "Awesome Sphinx Theme — Create functional and beautiful websites for your documentation with Sphinx." [2]: https://pypi.org/project/sphinxawesome-theme/5.2.0/#history [3]: https://github.com/kai687/sphinxawesome-theme/releases/tag/5.2.0
1c228c0
to
14a0fa6
Compare
Thank you @hakre! |
Close to a good share of a hand full of improvements of the php-src docs build.
Driven by my initial request in PHP-DEV for wiki editing right after which @cmb69 pointed me to the fact that there is more value in moving things towards php-src.
Therefore I looked into it and found
make html
not working addressed by adding a Makefile.Found a benefit in having the requirements in a file instead of scattered, henceforth added a requirements.txt file and bound it in all places I could find. (the build could perhaps benefit from opening to branches other than master for docs/**, only publishing should remain on master branch only.)
Would be delighted to get a review by @iluuu1994 a) for the overall thing and b) there is a deprecation in the theme configuration and the theme is new to me.