GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl by epicfaace · Pull Request #11447 · python/cpython

Conversation

@epicfaace

@epicfaace epicfaace commented Jan 6, 2019 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue12707

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
epicfaace added 3 commits January 6, 2019 10:44
@epicfaace
bpo-12707: deprecate info(), geturl(), getcode() from HTTPResponse an…
18a7eac 
…d addinfo
@epicfaace
bpo-12707: rename 'code' attribute to 'status'
2e93da4
@epicfaace
bpo-12707: Document properties and deprecation of methods
2db7e1f
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Jan 6, 2019
@the-knights-who-say-ni

the-knights-who-say-ni commented Jan 6, 2019

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Our records indicate we have not received your CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting review label Jan 6, 2019
@tirkarthi

tirkarthi commented Jan 6, 2019

Copy link
Copy Markdown
Member

I think there is no consensus on the issue about raising code level deprecation as noted in https://bugs.python.org/issue12707#msg234945 and this causes a lot of tests to fail to DeprecationWarning as seen in the CI. I hope it's better to split the documentation changes into a separate PR without 3.8 deprecation notice in docs and code for review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
epicfaace added 4 commits January 6, 2019 14:09
@epicfaace
bpo-21257: remove deprecation warnings
beadb34
@epicfaace
bpo-21257: fix whitespace
2a16f9b
@epicfaace
bpo-12707: remove deprecation warnings from doc
f07f456
@epicfaace
bpo-12707: remove deprecation warning for addurlinfo.getstatus() in docs
5640148
@epicfaace

epicfaace commented Jan 6, 2019

Copy link
Copy Markdown
Contributor Author

@tirkarthi Makes sense, I've removed the deprecation code/notice for now. It feels odd though having multiple methods/properties that return the same thing; ideally all the duplicates should be deprecated. For now I've added the "recommended to use [x] instead" notice in the docs.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@epicfaace
bpo-12707: rename code to status in test
51cd8ad
@epicfaace epicfaace changed the title bpo-12707: deprecate info(), geturl(), getcode() methods in favor of info, url, and status properties for HTTPResponse and addinfourl bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl Jan 7, 2019
@epicfaace
bpo-12707: rename info to headers in tests
c199098
@merwok
merwok self-requested a review January 7, 2019 02:22
eamanu
eamanu reviewed Jan 7, 2019
View reviewed changes
Comment thread Doc/library/http.client.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/http.client.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/urllib.request.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Jan 7, 2019
View reviewed changes
Comment thread Doc/library/http.client.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Jan 7, 2019
View reviewed changes
Comment thread Doc/library/http.client.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Jan 7, 2019
View reviewed changes
Comment thread Doc/library/urllib.request.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
eamanu and others added 3 commits January 7, 2019 08:28
@eamanu @epicfaace
Apply suggestions from code review
58ced25 
Co-Authored-By: epicfaace <[email protected]>
@epicfaace
bpo-12707: Add links to doc
13d373f
@epicfaace
bpo-12707 merge changes
357c8b4
merwok
merwok reviewed Jan 7, 2019
View reviewed changes
Comment thread Lib/test/test_urllib.py Outdated

def test_getcode(self):
self.assertIsNone(self.returned_obj.getcode())
self.assertIsNone(self.returned_obj.status)

@merwok merwok Jan 7, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think it could be cleaner to separate these tests (recommended attributes first, then the older ones)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 11, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok.

In other tests that use the info(), geturl() functions, etc., do you suggest I update them to use the recommended attributes as well too?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok May 22, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

How many changes is that?

If the functions are doc-deprecated but will never actually raise warnings, maybe it’s not worth changing tens and tens of lines.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
merwok
merwok reviewed Jan 7, 2019
View reviewed changes
Comment thread Lib/urllib/response.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@epicfaace
bpo-12707: remove empty line
b99fb9f
tirkarthi
tirkarthi reviewed Jan 8, 2019
View reviewed changes
Comment thread Doc/library/urllib.request.rst Outdated

.. attribute:: addinfourl.status

Status code returned by server. Recommended to use :attr:`~addinfourl.status` instead.

@tirkarthi tirkarthi Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

  • The recommendation phrase :attr:~addinfourl.status links back to itself while building the docs locally. Also below methods geturl, info also link to addinfourl.status but it's documented to return only status code. Am I missing something here?
  • status seems to be a newly added property. Does this need a versionadded directive?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ha this is recursive! Was probably copied from the getstatus docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Oops, that was a typo. Added a versionadded directive.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
merwok
merwok requested changes Jan 8, 2019
View reviewed changes
Comment thread Doc/library/http.client.rst Outdated

.. method:: HTTPResponse.geturl()

Returns the URL of the resource retrieved. Recommended to use :attr:`~HTTPResponse.url` instead.

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe this could be condensed to:

Deprecated method equivalent to :attr:`HTTPResponse.url`

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is that okay to call it "deprecated" even when it doesn't return any DeprecationWarnings when used?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Jan 9, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes, we name that «doc-deprecated», it’s a useful degree of deprecation.
(i.e. don’t break code that is fine, don’t annoy with warnings, just recommend the obvious way to do it in docs for people who want that)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 15, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Okay, I've went ahead and called it "deprecated" in the documentation. How do you keep track of which things mentioned in the documentation are deprecated and which are doc-deprecated?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/urllib.request.rst Outdated

.. attribute:: addinfourl.headers

Returns the meta-information of the page, such as headers,

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What does «such as» mean? Is there something else than HTTP response headers in the headers property?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Again, this was moved from the original documentation, but will reword.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated
.. attribute:: addinfourl.headers

Returns the meta-information of the page, such as headers,
in the form of an :func:`email.message_from_string` instance (see

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

See my previous comment: this should link to the EmailMessage class, not a factory function.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated

Returns the meta-information of the page, such as headers,
in the form of an :func:`email.message_from_string` instance (see
`Quick Reference to HTTP Headers <http://jkorpela.fi/http.html>`_)

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why link to a specific personal website here? I think people know what HTTP headers are, or if they don’t other parts of the docs probably already link to general-purpose documentation (W3C or Wikipedia).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Uh, I just moved over the previous documentation that was already there. Now that you mention it, it's probably better to remove it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Jan 9, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah! In that case, keeping it is also OK. You can’t fix all the docs in one PR :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated

.. attribute:: addinfourl.status

Status code returned by server. Recommended to use :attr:`~addinfourl.status` instead.

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
Status code returned by server. Recommended to use :attr:`~addinfourl.status` instead.
Same as *code*.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated

.. method:: addinfourl.geturl()

Returns the URL of the resource retrieved. Recommended to use :attr:`~addinfourl.status` instead.

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What do you think of:

Suggested change
Returns the URL of the resource retrieved. Recommended to use :attr:`~addinfourl.status` instead.
Deprecated method equivalent to :attr:`~addinfourl.url`.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated

.. method:: addinfourl.info()

Returns the response headers. Recommended to use :attr:`~addinfourl.status` instead.

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

headers not status!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/urllib.request.rst Outdated

.. method:: addinfourl.getstatus()

Returns the status. Recommended to use :attr:`~addinfourl.status` instead.

@merwok merwok Jan 8, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should status or code be the obvious recommended attribute?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Since originally both HTTPResponse and getaddrurl had the same three functions, info(), geturl(), and getcode(), and now we're replacing them with attributes instead. Both classes have .headers and .url so we're good with those two. However, there is no uniform attribute for the status code; we have HTTPResponse.status and addinfourl.code.

Since it seems like HTTPResponse.status is already documented, I thought it would be better just to rename addinfourl.code to addinfourl.status for consistency's sake.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Jan 9, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Agreed that following HTTPResponse is best.

Suggested doc for status property: Returns the status code.

Doc for code: Same as status

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Jan 9, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry I am confused: does addinfourl.code already exist in a released version? If yes, I think we should keep it, recommend status, and not add code to HTTPResponse. If no, let’s keep only getcode (old) and status (recommended, same name as other class). Sounds good?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@epicfaace epicfaace Jan 9, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Oops, I made a typo. What I meant to say is that: HTTPResponse.status and addinfourl.code already both exist in a released version.

So my solution was to add a property called addinfourl.status which would be recommended to use over addinfourl.code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
merwok reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Lib/urllib/response.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting review label Jan 8, 2019
@bedevere-bot

bedevere-bot commented Jan 8, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella csabella reopened this May 22, 2019
@epicfaace

epicfaace commented Aug 19, 2019

Copy link
Copy Markdown
Contributor Author

@csabella @merwok any updates on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@blurb-it
📜🤖 Added by blurb_it.
2ee88dd
merwok
merwok approved these changes Aug 27, 2019
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Aug 27, 2019
matrixise
matrixise suggested changes Sep 11, 2019
View reviewed changes

@matrixise matrixise left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you for your contribution but could you update your PR with my remarks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Documentation/2019-08-27-01-14-59.bpo-12707.Yj3_7_.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/urllib.request.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/http.client.rst Outdated

.. method:: HTTPResponse.geturl()

Deprecated method equivalent to :attr:`HTTPResponse.url`.

@matrixise matrixise Sep 11, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could you use

.. deprecated:: 3.9
    The method :method:`HTTPResponse.geturl` is deprecated in favor of :attr:`HTTPResponse.url`

Check for the other attributes and the right syntax.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@merwok merwok Sep 11, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I disagree! Why generate a link to the section that we’re already in?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
matrixise reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@epicfaace epicfaace Sep 13, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, I've done something in between by starting the sentence just with "Deprecated in favor" while still keeping the deprecated:: markup.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot

bedevere-bot commented Sep 11, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting merge labels Sep 11, 2019
epicfaace added 2 commits September 13, 2019 04:24
@epicfaace
Make NEWS more clear
d7575f0
@epicfaace
add Deprecated warnings and update versionadded
628c2f0
@epicfaace

epicfaace commented Sep 13, 2019

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Sep 13, 2019
@bedevere-bot

bedevere-bot commented Sep 13, 2019

Copy link
Copy Markdown

Thanks for making the requested changes!

@merwok, @matrixise: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested review from matrixise and merwok September 13, 2019 04:26
matrixise
matrixise previously approved these changes Sep 13, 2019
View reviewed changes

@matrixise matrixise left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you for your contribution

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Sep 13, 2019
@matrixise
Fix the call of the 'deprecated' directive
5f5b2f9
@matrixise

matrixise commented Sep 13, 2019

Copy link
Copy Markdown
Member

@epicfaace I have fixed the .. deprecated:: 3.9 in your branch, I am waiting for the green lights from Travis and I will merge it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matrixise
matrixise dismissed their stale review September 13, 2019 11:12

I have seen an error with the deprecated directive and I can't compile the doc on my laptop

@matrixise
matrixise merged commit ff2e182 into python:master Sep 13, 2019
@bedevere-bot bedevere-bot removed the awaiting merge label Sep 13, 2019
@matrixise

matrixise commented Sep 13, 2019

Copy link
Copy Markdown
Member

Thank you so much for your contribution, I closed the issue and have merged this PR.
merwok reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk hugovk mentioned this pull request Jan 24, 2023
Docs: Fix getstatus() -> getcode() typos #101296
Merged
@hugovk

hugovk commented Feb 5, 2023

Copy link
Copy Markdown
Member

This PR documented functions returning a status code as getstatus(), when they're really getcode().

Please could someone review PR #101296 to fix it?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@dimo414 dimo414 mentioned this pull request Jan 9, 2024
Usage of HTTPResponse.url #86228
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tirkarthi tirkarthi tirkarthi left review comments
@merwok merwok Awaiting requested review from merwok
+3 more reviewers
@eamanu eamanu eamanu left review comments
@ZackerySpytz ZackerySpytz ZackerySpytz left review comments
@matrixise matrixise matrixise left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@epicfaace @the-knights-who-say-ni @tirkarthi @bedevere-bot @csabella @matrixise @hugovk @merwok @eamanu @ZackerySpytz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.