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

Is Content-Type optional? #2500

Closed
encukou opened this issue Apr 4, 2022 · 10 comments · Fixed by #3454
Closed

Is Content-Type optional? #2500

encukou opened this issue Apr 4, 2022 · 10 comments · Fixed by #3454
Assignees
Labels
infra Core infrastructure for building and rendering PEPs question

Comments

@encukou
Copy link
Member

encukou commented Apr 4, 2022

PEP 1 lists the Content-Type header as optional, but the linter checks for its presence. Which is correct?

(The other mismatches are Discussions-To and Post-History, but those typically need to be added after a PEP is published.)

@CAM-Gerlach
Copy link
Member

See #2355 , specifically this thread for some context on this.

Personally, for the reasons mentioned there, and especially after #2376 and the removal of the legacy build system, I feel its best to simply things and just elide the header from PEP 1, PEP 12 and the template since it is simply excess boilerplate that doesn't actually do anything anymore (it can stay in existing PEPs, like several other legacy headers). If we do support a new PEP format in the future (e.g. MyST), it would be indicated via extension in the source, and be transparent in the rendered output, and if for some reason we did need to add it back (its already simply ignored in the build backend), we could simply consider text/x-rst to be its default value if not provided, exactly as we did for text/plain in the past, so there's no backward-compat harm in not requiring it now.

@CAM-Gerlach CAM-Gerlach added question infra Core infrastructure for building and rendering PEPs labels Apr 6, 2022
@hugovk
Copy link
Member

hugovk commented Apr 7, 2022

Shall we remove Content-Type when updating old PEPs?

I'm not suggest we ask others to do so in their PRs, but we could as we're doing other updates.

For example I was going to update the release date in PEP 644, and could remove Content-Type at the same time.

@JelleZijlstra
Copy link
Member

I would not remove it, in the spirit of keeping unnecessary changes to a minimum.

@CAM-Gerlach CAM-Gerlach self-assigned this Apr 16, 2022
@CAM-Gerlach
Copy link
Member

If we all agree that it should not be required for new PEPs, I can update the relevant bits of PEP 1, PEP 12 and the template, as well as the linter check...

@AA-Turner
Copy link
Member

I'm still -0 on removing it as it is useful for explicitness (the Docutils project continues to reccommend .txt for reST content!), but if the general consensus is that it should go then fair enough. There's no technical reason for the header anymore in truth.

We should be consistent between the PEP 1, 12, and the automatic checks regardless of the outcome, though.

A

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Apr 16, 2022

I'm still -0 on removing it as it is useful for explicitness

We're not really "removing" it any more than it already is; nothing would change in the modern build system (which AFAIK has always ignored it and thrown it away on injest) or existing PEPs, just PEP 1, 12 the template and the linters would stop requiring it be specified explicitly for new PEPs.

And useful how, by what? As mentioned, it is not used with the current build system, which is the only tool that directly consumes the raw PEPs, and not exposed in the experimental public API. And further, if for some reason it is ever needed in the future, it can be simply treated as having a default value of text/x-rst, exactly as it was during the era of the legacy build system, without authors having to add an extra line of boilerplate to every PEP until then.

(the Docutils project continues to reccommend .txt for reST content!

This might have made some sense two decades ago when it was created, but is wholly out of step with modern practices and tooling (Git, GitHub, editors, code highlighters, linters, Sphinx itself, etc) and regardless is not what Sphinx recommends or defaults to. We've all seen the problems and the ambiguity it causes in terms of determining what files to process (and lint, etc), and the lengths we went to mitigate that, which adds non-trivial complexity in various places. If we ever do switch to a new format, it would almost certainly have a new file extension; by far the most likely candidate would be MyST, which (AFAIK) explicitly relies on the file extension for Sphinx to correctly determine how to parse each file and allow the two types to co-exist seamlessly.

@AA-Turner
Copy link
Member

useful how, by what

As a marker that the content is reST. For older PEPs, the extension is .txt, which gives no indication as to the content type -- whilst we all know it is reST, it is an explicit orientation device.

As I said, there is no programmatic use-case anymore, and the justification above is weak, but given copying an existing PEP or using the template to start draughting a new PEP would both have the header, I don't see it as much burden to keep including it.

I'll have this be my last comment on this issue.

A

P.S. I'm well aware of the .rst vs .txt issue, I've suggested updating it to Docutils but there was limited enthusiasm.

@CAM-Gerlach
Copy link
Member

As a marker that the content is reST. For older PEPs, the extension is .txt, which gives no indication as to the content type -- whilst we all know it is reST, it is an explicit orientation device.

Well, again, we're not proposing changing any of the older PEPs, and it seems unlikely that users would miss the mentions in the Readme, contributing guide, PEP docs, PEP 1, PEP 12, template, etc. but still read and parse it out of the source headers, but eh.

P.S. I'm well aware of the .rst vs .txt issue, I've suggested updating it to Docutils but there was limited enthusiasm.

Hmm, well, I suppose its not so relevant what they say anymore—now that we've switched, does any significant project use bare docutils as opposed to Sphinx (aside from maybe the PyPI readme rendered)?

@hugovk
Copy link
Member

hugovk commented Apr 16, 2022

If we all agree that it should not be required for new PEPs, I can update the relevant bits of PEP 1, PEP 12 and the template, as well as the linter check...

+0.5

I think it would a good idea to make it optional for new PEPs with a default of text/x-rst, and update these things.

If someone copies and pastes an old PEP to create a new one, fine, either way it's not a problem to keep the field or suggest it removed.

@hugovk
Copy link
Member

hugovk commented Sep 27, 2023

Please see PR #3454.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Core infrastructure for building and rendering PEPs question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@encukou @JelleZijlstra @hugovk @AA-Turner @CAM-Gerlach and others