Skip to content

gh-93166: Clarify http.server documentation using RFC 7230 terminology #30686

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

Closed
wants to merge 13 commits into from

Conversation

geryogam
Copy link
Contributor

@geryogam geryogam commented Jan 19, 2022

@MaxwellDupre
Copy link
Contributor

A nice piece of work to tidy this up. Only minor changes please:
Line 210:
The *code* specifies the status code, *message* the reason phrase,

Line 274:
The *code* specifies the status code

Adding The at the start of these lines. Otherwise it looks strange, since the italicisation is not that pronounced and looks like there is something missing.

@erlend-aasland
Copy link
Contributor

This is way too large a change to pass as skip issue. Please file an issue for this change.

Copy link
Member

@merwok merwok left a comment

Choose a reason for hiding this comment

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

I made a few notes on the first changes in the PR.

CPython is big and old, and as we’ve discussed in other PRs, sweeping changes are weighed cautiously. There is also no goal to apply a very specific style to all parts of the documentation; it’s fine if different documents contributed by different authors may have different styles, as long as it fits in its overall doc section.

To apply your time and energy in a way that works for CPython reviewers and gives more satisfactory results for you, I would recommend that you engage more before making edits: create tickets with specific points to be improved, and/or discuss these points on discuss. You could even join the Documentation community team to take part in tackling the big problems in Python’s doc! https://docs-community.readthedocs.io/en/latest/community/index.html

@@ -24,7 +24,7 @@ This module defines classes for implementing HTTP servers.

One class, :class:`HTTPServer`, is a :class:`socketserver.TCPServer` subclass.
It creates and listens at the HTTP socket, dispatching the requests to a
handler. Code to create and run the server looks like this::
handler. Code to create and run the server looks like this::
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid this kind of changes. They do not fix or improve anything, so just churn history for not benefit.


.. attribute:: path

Contains the request path. If query component of the URL is present,
The request target. If query component of the URL is present,
Copy link
Member

Choose a reason for hiding this comment

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

Let’s not replace path with target here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not the path component of a URI whose syntax is defined in RFC 3986, it is the request-target component of an HTTP request line whose syntax is defined in RFC 7230. The other documented attributes command (method) and request_version (HTTP-version) of BaseHTTPRequestHandler already use the syntactic terminology of an HTTP request line.

then ``path`` includes the query. Using the terminology of :rfc:`3986`,
``path`` here includes ``hier-part`` and the ``query``.

.. attribute:: request_version

Contains the version string from the request. For example, ``'HTTP/1.0'``.
The request HTTP version. For example, ``'HTTP/1.0'``.
Copy link
Member

Choose a reason for hiding this comment

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

Previous wording was more elegant (except contains)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am using RFC 7230 terminology.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think the original version is more readable and is consistent with RFC, and provides additional information that it is a string.

@geryogam geryogam changed the title Update http.server.rst Clarify http.server documentation using RFC 7230 terminology May 24, 2022
@geryogam geryogam changed the title Clarify http.server documentation using RFC 7230 terminology gh-93166: Clarify http.server documentation using RFC 7230 terminology May 24, 2022
@merwok merwok linked an issue May 24, 2022 that may be closed by this pull request
@geryogam
Copy link
Contributor Author

This is way too large a change to pass as skip issue. Please file an issue for this change.

@erlend-aasland Done.

@erlend-aasland
Copy link
Contributor

Please remove all changes not directly related to the issue, as previously requested by Éric in #30686 (review).

@erlend-aasland erlend-aasland marked this pull request as draft March 6, 2023 12:50
@erlend-aasland
Copy link
Contributor

I've marked this PR as a draft since it is not ready for review.

@AA-Turner
Copy link
Member

Closing due to inactivity. If you'd like to have this change merged, please update per Éric and Erlend's requests.

A

@AA-Turner AA-Turner closed this Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify http.server documentation using RFC 7230 terminology