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

Run Sphinx generation in Python 3. #26574

Merged
merged 8 commits into from Nov 26, 2020
Merged

Conversation

ziransun
Copy link
Member

Issue #23081

This CL made the following changes:
[1] Upgrading Sphinx version to 3.3.1 to support python 3.5+
[2] Fixing bugs invoked during Py3 run.
a) Replace reference of "index" in checklist.md with relative path for
index.md. This is due to "more than one target found for cross-reference"
error thrown by Sphinx referring to it. It's a known issue in
sphinx (sphinx-doc/sphinx#2549)
b) Fix Sphinx in third_party/pywebsocket3. A PR has been sent at
GoogleChromeLabs/pywebsocket3#16

Note: Command to run with Python3 is: wpt --py3 build-docs

Issue web-platform-tests#23081

This CL made the following changes:
[1] Upgrading Sphinx version to 3.3.1 to support python 3.5+
[2] Fixing bugs invoked during Py3 run.
    a) Replace reference of "index" in checklist.md with relative path for
       index.md. This is due to "more than one target found for cross-reference"
       error thrown by Sphinx referring to it. It's a known issue in
       sphinx (sphinx-doc/sphinx#2549)
    b) Fix Sphinx in third_party/pywebsocket3. A PR has been sent at
       GoogleChromeLabs/pywebsocket3#16

Note: Command to run with Python3 is: wpt --py3 build-docs
@Hexcles
Copy link
Member

Hexcles commented Nov 19, 2020

The job failed on GitHub Actions: https://github.com/web-platform-tests/wpt/pull/26574/checks?check_run_id=1424211186#step:3:414

Looks like we don't have virtualenv in Docker? The change looks sane to me. I'm not sure why it worked before but not now; I suspect it's the difference between the base images (python:2-stretch vs python:3.6-stretch).

Copy link
Member

@Hexcles Hexcles left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. This now mostly looks good except some minor comments.

docs/conf.py Outdated Show resolved Hide resolved
@@ -1,4 +1,4 @@
recommonmark==0.6.0
# pin this to the last Py2 release
Sphinx==1.8.5 # pyup: <2.0
Sphinx==3.3.1 # pyup: <2.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Sphinx==3.3.1 # pyup: <2.0
Sphinx==2.4.4 # pyup: <3.0

docs/requirements.txt Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@ziransun
Copy link
Member Author

@Hexcles: Code updated. Thank you!

Co-authored-by: Robert Ma <bob1211@gmail.com>
docs/requirements.txt Outdated Show resolved Hide resolved
tools/docker/documentation/Dockerfile Outdated Show resolved Hide resolved
docs/requirements.txt Outdated Show resolved Hide resolved
docs/requirements.txt Outdated Show resolved Hide resolved
@Hexcles Hexcles requested a review from jgraham November 26, 2020 19:32
@Hexcles
Copy link
Member

Hexcles commented Nov 26, 2020

@jgraham PTAL again. Also, WDYT about the pywebsocket3 change? This is the only change since our latest roll. I'm happy to do a subtree pull --squash directly on master, or we could just merge this PR as is.

@Hexcles Hexcles merged commit da46f68 into web-platform-tests:master Nov 26, 2020
Hexcles added a commit that referenced this pull request Dec 14, 2020
1. Mark the command as py3only (since #26574).
2. Skip .tox, and fix other exclude patterns.
@Hexcles Hexcles mentioned this pull request Dec 14, 2020
Hexcles added a commit that referenced this pull request Dec 15, 2020
1. Mark the command as py3only (since #26574).
2. Skip .tox, and fix other exclude patterns.
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.

None yet

6 participants