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

Update the MPS Manual for compatibility with Sphinx 3. #67

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

gareth-rees
Copy link
Member

No description provided.

@gareth-rees gareth-rees linked an issue May 28, 2021 that may be closed by this pull request
@rptb1
Copy link
Member

rptb1 commented Jan 11, 2023

I was able to build the manual on Ubuntu 22 with Sphinx 6.1.3 without errors:

cd manual
sudo apt-get install virtualenv
make tools
make SPHINXBUILD=tool/bin/sphinx-build html

however something's wrong with the layout of the result (Firefox 108.0.2):
image

@rptb1
Copy link
Member

rptb1 commented Jan 18, 2023

something's wrong with the layout of the result

This also occurs in master at 7fb1ad6 with Sphinx 6.1.3 so there's no reason to think it was introduced by this work.

I'm able to remove the layout problem by installing Sphinx 4, so let's specify Sphinx < 5 and make a separate issue for the problem.

…ntly broken under Sphinx 5 and 6 <#121>.  Because that means we're using a variant of Sphinx, also making a venv-installed Sphinx the default.
@rptb1
Copy link
Member

rptb1 commented Jan 18, 2023

Just for the record, I tried tool/bin/pip install 'Sphinx < 3.6'. It wouldn't run on my Ubuntu 22, so I can't easily check whether this branch pedantically meets its stated purpose, but I don't think that matters now.

Copy link
Member

@rptb1 rptb1 left a comment

Choose a reason for hiding this comment

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

These changes look good.

I did the following steps:

  1. Checked I could build the manual locally (results and changes are in the pull request thread Update the MPS Manual for compatibility with Sphinx 3. #67 ).
  2. Read the diffs, focussing on multi-line diffs, but sampling others at random.
  3. Sampled pages of the built manual that changed at random and checked them against the diffs.
  4. Checked manual/html/design/protocol.html because it could be affected by a mistake in the new doc code. It wasn't.
  5. Read the new paragraphs of manual/html/design/doc.html relating to these changes to make sure they explained the new convention. They did.

I created #121 and #122 as a result of this review but they are not issues with this work.

manual/source/extensions/mps/designs.py Show resolved Hide resolved
@rptb1 rptb1 added the build Problems with builds and build automation label Jan 18, 2023
@rptb1
Copy link
Member

rptb1 commented Jan 19, 2023

Executing pull request merge procedure.

Checklist results:
2. Branch solves related problem.
3. No automated test case is feasible for this. It would need to try to build the manual with an earlier Sphinx and fail. Unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Problems with builds and build automation documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manual is not compatible with Sphinx 3.5
2 participants