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

Support code highlighting #1577

Merged
merged 5 commits into from
Nov 23, 2021
Merged

Support code highlighting #1577

merged 5 commits into from
Nov 23, 2021

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Aug 27, 2020

See: #1571 (comment)

Fine to try it in another PR, but looks like all that really is needed is to install pygments:

  1. The script step calls make to turn plaintext PEPs into HTML, and I assume pygments is only needed here to add colour to the generated HTML files (I'm afk this week so cannot verify this assumption.now).
  2. Then the deploy step calls deploy.bash which does make package to tar.gz the plaintext and generated files and images, and then calls aws to actually deploy the tar.gz to AWS.

@brettcannon
Copy link
Member

Have you checked if this will work with the deployment of the PEPs on python.org?

@ofek
Copy link
Contributor Author

ofek commented Aug 27, 2020

How would one test that?

@Mariatta
Copy link
Member

The process to build the PEPs from python.org is described here https://pythondotorg.readthedocs.io/pep_generation.html

@ofek
Copy link
Contributor Author

ofek commented Aug 28, 2020

django.db.utils.OperationalError: could not connect to server: No such file or directory
        Is the server running locally and accepting
        connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?

@ofek
Copy link
Contributor Author

ofek commented Aug 28, 2020

I'm having difficulty building the site

@ofek
Copy link
Contributor Author

ofek commented Sep 2, 2020

Could one of you please try?

@brettcannon
Copy link
Member

We don't 'have any experience with the web site, so it would be best to ask on the pythondotorg repo instead of here.

@hugovk
Copy link
Member

hugovk commented Sep 2, 2020

I was also unsuccessful, I tried both the Vagrant and manual methods from https://pythondotorg.readthedocs.io/install.html

@Mariatta
Copy link
Member

Mariatta commented Sep 4, 2020

Please report issue wrt pythondotorg guide in the GitHub.com/Python/pythondotorg repo

@ofek
Copy link
Contributor Author

ofek commented Sep 9, 2020

python/pythondotorg#1633

@ofek
Copy link
Contributor Author

ofek commented Sep 18, 2020

@brettcannon Seems to work python/pythondotorg#1633 (comment)

@ofek ofek requested a review from a team as a code owner June 2, 2021 02:34
@ofek
Copy link
Contributor Author

ofek commented Jun 2, 2021

I rebased, should be good to merge now

@@ -17,7 +17,7 @@ jobs:
- name: Install dependencies
run: |
python -m pip install -U pip
python -m pip install -U docutils
python -m pip install -U docutils pygments
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should create requirements.txt and pin the versions ofdocutils and pygments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member

Choose a reason for hiding this comment

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

Perils of reading commit info from emails and not checking the latest! Feel free to bump the docutils/sphix reqs, they are current as of April 2020, so 0.17.z / 4.y.z may be more appropriate.

A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed commit!

docutils >= 0.16
Pygments ~= 2.9.0
Sphinx ~= 4.0.2
docutils ~= 0.17.1

Copy link
Member

@AA-Turner AA-Turner Jun 19, 2021

Choose a reason for hiding this comment

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

Only question I'd have is if better to pin to compatible releases or all future? (~= vs >=) - historically been unpinned so I think that means the latest release has always been used.

(but I am not best placed to make this call, as no idea if e.g. previously there have been breaking changes to PEP rendering due to dependency changes, or the risk appetite for this in the future vs new features that may or may not be used)

A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, changed it back

@ofek
Copy link
Contributor Author

ofek commented Jun 21, 2021

all good to merge?

@ofek
Copy link
Contributor Author

ofek commented Jun 23, 2021

is there any other concern here?

@brettcannon
Copy link
Member

@ofek matter of time to look at the PR (which I have), deciding if we want it, and then watching python.org/dev/peps to make sure nothing breaks.

@ofek
Copy link
Contributor Author

ofek commented Nov 21, 2021

Any update on this?

@brettcannon
Copy link
Member

None from me, but I'm not the only PEP editor. I suspect we are all just busy with other stuff at the moment.

@JelleZijlstra
Copy link
Member

Since it's been so long I'm going to re-trigger CI just to make sure. If it's still good I'm for merging this though.

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.

7 participants