Closed emanlove closed 6 days ago
β±οΈ Estimated effort to review [1-5] | 3 |
π§ͺ Relevant tests | No |
π Security concerns | No |
β‘ Key issues to review |
Documentation Structure: The PR involves significant changes to the documentation structure, including the removal of several .rst files and the addition of new modules in the API documentation. It's crucial to ensure that all new modules are correctly documented and that no essential information is lost in the transition. |
Sphinx Version Compatibility: The update to a newer Sphinx version (from 1.8.2 to 7.1.2) may introduce changes in how documents are built or displayed. It's important to verify that the new Sphinx version is fully compatible with the existing documentation setup and that the documentation builds without errors or significant changes in formatting. |
Category | Suggestion | Score |
Best practice |
Ensure all new modules are correctly linked and documented___ **Verify that all new modules added to theapi.rst file are correctly linked and documented in their respective .rst files to ensure proper documentation generation.**
[py/docs/source/api.rst [79-103]](https://github.com/SeleniumHQ/selenium/pull/14173/files#diff-7e8f0a6376548281d6c1a5b57121f0a701a76c5f88129fe7def3d89f39b63c43R79-R103)
```diff
+selenium.webdriver.chrome.remote_connection
+selenium.webdriver.chromium.service
+selenium.webdriver.edge.remote_connection
-
```
- [ ] **Apply this suggestion**
Suggestion importance[1-10]: 8Why: This suggestion is crucial for ensuring that the documentation is complete and functional, preventing broken links and ensuring that all new modules are accessible and well-documented. | 8 |
Maintainability |
Ensure consistent formatting and alignment for new module entries___ **Ensure that all new modules added to theapi.rst file are correctly formatted and aligned with the existing entries to maintain consistency and readability.** [py/docs/source/api.rst [27-37]](https://github.com/SeleniumHQ/selenium/pull/14173/files#diff-7e8f0a6376548281d6c1a5b57121f0a701a76c5f88129fe7def3d89f39b63c43R27-R37) ```diff +selenium.webdriver.common.driver_finder +selenium.webdriver.common.options +selenium.webdriver.common.selenium_manager +selenium.webdriver.common.utils +selenium.webdriver.common.virtual_authenticator - ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: The suggestion is relevant as it addresses the maintainability and readability of the documentation by ensuring new entries are well-formatted and aligned. This is important for large documentation files to remain manageable and user-friendly. | 7 |
Possible issue |
Add a check to ensure
___
**Add a check to ensure that the | 6 |
If one wants to easily test or reproduce the steps this is doing, the following commands will setup an virtualenv, grab the latest code and run the doc generation through tox; similar to what the official build does.
virtualenv test-py38-env
source test-py38-env/bin/activate
pip install tox
git clone git@github.com:emanlove/selenium.git se-api-docs-test
cd se-api-docs-test/
git switch -c py-api-doc-13910 origin/py-api-doc-13910
tox -c py/tox.ini
Oh wow, yeah, this looks great. When looking at the diff I'm seeing several things in the current docs that still reference Selenium 4.17
@emanlove can you fix the conflict (looks like adding the line from #14171 isn't part of this PR)
Also can you create another issue for auto-generating the api.rst file so we can track it?
@titusfortner Fixed the conflict. Sorry about that one.
@iampopovich I know you did the work to update the api.rst file here, do you want me to merge your file and rebase this on top of it, or can I just merge this? Also are you in our chat room? (https://www.selenium.dev/support/#ChatRoom) We can discuss more there if you'd like.
These changes are better than mine. It's fine to use them, no worries. They also include part of my work, so it's all fair π€
Now I just need to see about retroactively generating docs from the 4.22 tagπ
User description
Description
These changes regenerate the api stub files - those template files which give the base structure for the api docs - each time instead of using outdated checked in version from previous builds. Essentially we treat these like new builds which need to be regenerated versus updating and storing old build artifacts.
The base API template (py/docs/source/api.rst) has been updated to match current high level python modules which the api doc is built from. Ideally this should be autogenerated itself simply matching the Python codebase. Leaving that for another body of work to do sometime in the future.
Also updated the versions of the documentation tool (sphinx) and template packages (jinja) to more recent version. (Due to using Python 3.8 the latest sphinx version can't be used but this is ok.).
Motivation and Context
The API changes with each release as it matches the code. So essentially each time a new release comes out a new build of the documentation should be generated.
Types of changes
Checklist
PR Type
Bug fix, Enhancement
Description
py/docs/requirements.txt
from 1.8.2 to 7.1.2 to support Python 3.8.py/docs/source/api.rst
by adding new modules and reordering them alphabetically.py/tox.ini
to usedocs/requirements.txt
for dependencies, added a step to regenerate autodoc stub pages, and set thePYTHONPATH
environment variable.Changes walkthrough π
requirements.txt
Update Sphinx version in documentation requirements
py/docs/requirements.txt - Updated Sphinx version from 1.8.2 to 7.1.2.
api.rst
Update API documentation structure and modules
py/docs/source/api.rst
tox.ini
Update tox configuration for documentation build
py/tox.ini
docs/requirements.txt
for dependencies.PYTHONPATH
environment variable.