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.

Sign up for GitHub

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

Jump to bottom

Update http.client.rst #24803

Merged
merged 4 commits into from
Oct 3, 2022
Merged

Update http.client.rst #24803

merged 4 commits into from
Oct 3, 2022

Conversation

geryogam
Copy link
Contributor

@geryogam geryogam commented Mar 9, 2021 •
edited
Loading

Fix some typos in the documentation of the http.client standard library.

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Mar 9, 2021
@github-actions
Copy link

github-actions bot commented Apr 9, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 9, 2021
merwok
merwok reviewed Jan 5, 2022
View reviewed changes
Doc/library/http.client.rst
@@ -572,7 +572,7 @@ Here is an example session that uses the ``HEAD`` method. Note that the
>>> data == b''
True

Here is an example session that shows how to ``POST`` requests::
Here is an example session that uses the ``POST`` method::
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

merwok
merwok reviewed Jan 5, 2022
View reviewed changes
Doc/library/http.client.rst

>>> # This creates an HTTP message
>>> # This creates an HTTP request
Copy link
Member

@merwok merwok Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

request is the abstract concept, but message is a type in this module, and used just below in the doc.
This line was not incorrect, I suggest reverting the change.

merwok
merwok reviewed Jan 5, 2022
View reviewed changes
Doc/library/http.client.rst Outdated Show resolved Hide resolved
geryogam and others added 2 commits January 5, 2022 10:03
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 5, 2022
@slateny
Copy link
Contributor

slateny commented Oct 3, 2022

@maggyero Would you still be interested in addressing the comments above?

orsenthil
orsenthil approved these changes Oct 3, 2022
View reviewed changes
Copy link
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

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

LGTM.

The only critical comment was reverting to message instead of request. but within the context, the usage of request was fine.

@orsenthil orsenthil added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Oct 3, 2022
@orsenthil orsenthil closed this Oct 3, 2022
@orsenthil orsenthil reopened this Oct 3, 2022
@orsenthil orsenthil merged commit 0c91a12 into python:main Oct 3, 2022
@miss-islington
Copy link
Contributor

Thanks @maggyero for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-97808 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 3, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 3, 2022
@geryogam @miss-islington
Update http.client.rst (pythonGH-24803)
84b3df3 
* Update http.client.rst

* Apply suggestions from code review

Co-authored-by: Éric <merwok@netwok.org>

* Update http.client.rst

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Senthil Kumaran <senthil@python.org>
(cherry picked from commit 0c91a12)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
@bedevere-bot
Copy link

GH-97809 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 3, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 3, 2022
@geryogam @miss-islington
Update http.client.rst (pythonGH-24803)
d9d2a2d 
* Update http.client.rst

* Apply suggestions from code review

Co-authored-by: Éric <merwok@netwok.org>

* Update http.client.rst

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Senthil Kumaran <senthil@python.org>
(cherry picked from commit 0c91a12)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
@merwok
Copy link
Member

merwok commented Oct 3, 2022

Some of my questions were left unanswered before this was merged, but it’s not very important.

For other PRs of the same style, please follow these guidelines:

  • open an issue first, or a forum discussion
  • propose substantive changes that do improve the text
  • for cosmetic or debatable changes, get agreement on the issue first
  • write a meaningful PR title
  • address reviews

Thanks!

miss-islington added a commit that referenced this pull request Oct 3, 2022
@miss-islington @geryogam
Update http.client.rst (GH-24803)
789d5bc 
* Update http.client.rst

* Apply suggestions from code review

Co-authored-by: Éric <merwok@netwok.org>

* Update http.client.rst

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Senthil Kumaran <senthil@python.org>
(cherry picked from commit 0c91a12)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
miss-islington added a commit that referenced this pull request Oct 3, 2022
@miss-islington @geryogam
Update http.client.rst (GH-24803)
6569837 
* Update http.client.rst

* Apply suggestions from code review

Co-authored-by: Éric <merwok@netwok.org>

* Update http.client.rst

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Senthil Kumaran <senthil@python.org>
(cherry picked from commit 0c91a12)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
@orsenthil
Copy link
Member

Hi @merwok ,

I made a note regarding your comment while reviewing it.

The only critical comment was reverting to message instead of request. but within the context, the usage of request was fine.

Didn't want to keep the PR with other changes in open for a long time since your other comments were addressed. I acknowledge your other points.

Thank you.

@merwok
Copy link
Member

merwok commented Oct 4, 2022

Good, and no worry!

pablogsal pushed a commit that referenced this pull request Oct 22, 2022
@miss-islington @geryogam @pablogsal
Update http.client.rst (GH-24803)
fb0d931 
* Update http.client.rst

* Apply suggestions from code review

Co-authored-by: Éric <merwok@netwok.org>

* Update http.client.rst

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Senthil Kumaran <senthil@python.org>
(cherry picked from commit 0c91a12)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merwok merwok merwok left review comments

@orsenthil orsenthil orsenthil approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@geryogam @slateny @miss-islington @bedevere-bot @merwok @orsenthil @the-knights-who-say-ni @ezio-melotti