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

Sphinx - core #1930

Merged
merged 10 commits into from
Jun 8, 2021
Merged

Sphinx - core #1930

merged 10 commits into from
Jun 8, 2021

Conversation

AA-Turner
Copy link
Member

See #2, #1385 for context. Superseeds #1565.

This is the minimal core Sphinx support part, adding a bare minimum of useful things to get Sphinx to build and deploy, whilst not affecting the current build system. There is no theming or custom parsing needed to properly deal with PEPs.

  • build.py - build script
  • conf.py - Sphinx configuration
  • Makefile - new targets for Sphinx
  • .gitignore - add ignores for venv and package directories
  • contents.rst - Sphinx page to discover all PEPs
  • deploy-gh-pages.yaml - builds and deploys to github pages
  • requirements.txt


# New Sphinx targets:

SPHINX_JOBS=8
Copy link
Member

Choose a reason for hiding this comment

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

Why 8?

Copy link
Member Author

Choose a reason for hiding this comment

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

Arbitrary number, beyond that diminishing returns. This was tested on GHA CI infrastructure, so a different number may be faster locally.

I'm considering disabiling search capabilities, as people will likely be better served with site:peps.python.org blah searches, and I think that the word stemming process takes a bit of time. Sphinx builds on CI took around 7 minutes, which does seem quite long (3-4x straight docutils)

(From #1934 (comment) - the makefile changes there come from this core PR)

@@ -0,0 +1,41 @@
name: Deploy to GitHub Pages
Copy link
Member

Choose a reason for hiding this comment

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

@AA-Turner I think we don't deploy on the GitHub pages... What's the reason for this workflow?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't currently. #1385 has extensive discussion on this (#1385 (comment) etc) - the main drive is that deploying to GH pages is easy and free, and enables previewing the changes before any switchovers etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also discussion in various other issues (#2, #3, #25) and others I've likely forgoton (may be some on the pythondotorg repo)

requirements.txt Outdated
docutils >= 0.16

# # For RSS
# feedgen >= 0.9.0 # For RSS feed
Copy link
Member

Choose a reason for hiding this comment

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

Why have you commented on the following lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

Half laziness in reserving the spaces for the RSS piece of work - should #1934 be merged the lines would be subsequently uncommented. If easier I could remove from this PR just for clarity?

Copy link
Member

Choose a reason for hiding this comment

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

Could you remove these lines and add them later in #1934? That way the commit is clear

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep will do

@pablogsal
Copy link
Member

@AA-Turner could you rebase this PR? That way I can approve it to run the CI

@pablogsal
Copy link
Member

Let's try to move this along, @AA-Turner could you please rebase this PR and address my comment? I can merge after that so we can continue with the rest

@AA-Turner
Copy link
Member Author

@pablogsal done & removed the extraneous stuff from requirements.txt

@pablogsal
Copy link
Member

@pablogsal done & removed the extraneous stuff from requirements.txt

Ok, I have unblocked the CI

@pablogsal pablogsal merged commit 3533799 into python:master Jun 8, 2021
@pablogsal
Copy link
Member

🚀 Ok, @AA-Turner, let's move on to the next one

pablogsal pushed a commit that referenced this pull request Jun 9, 2021
See #2, #1385 for context. Superseeds #1566.

This is the docutils parsing, transforms and writing part, building on PR #1930. It contains a pseudo-package, `sphinx_pep_extensions`, which itself contains:

### Docutils parsing:
- `PEPParser` - collates transforms and interfaces with Sphinx core
- `PEPRole` - deals with :PEP:`blah` in RST source

### Docutils transforms:
- `PEPContents` (Creates table of contents without page title)
- `PEPFooter` (Dels with footnotes, link to source, last modified commit)
- `PEPHeaders` (Parses RFC2822 headers)
- `PEPTitle` - Creates document title from PEP headers
- `PEPZero` - Masks email addresses and creates links to PEP numbers from tables in `pep-0000.rst`

### Docutils HTML output:
- `PEPTranslator` - Overrides to the default HTML translator to enable better matching of the current PEP styles
pablogsal pushed a commit that referenced this pull request Jun 15, 2021
See #2, #1385 for context.

This is the RSS generation part, building on PR #1930. It contains the logic for generating RSS

This was originally in #1385 and #1565, split out for ease of review
pablogsal pushed a commit that referenced this pull request Jun 30, 2021
See #2, #1385 for context. Superseeds #1568.

This is the Sphinx-theming part, building on PR #1930.

### Stylesheets:
- `style.css` - styles
- `mq.css` - media queries

### Jinja2 Templates:
- `page.html` - overarching template

### Javascript:
- `doctools.js` - fixes footnote brackets

### Theme miscellany
- `theme.conf` - sets pygments styles, theme internals
@AA-Turner AA-Turner deleted the sphinx-core branch July 4, 2021 18:55
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.

4 participants