◐ Shell
reader mode source ↗
Skip to content

bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl#11447

Merged
matrixise merged 31 commits into
python:masterfrom
epicfaace:bpo-12707
Sep 13, 2019
Merged

Conversation

@epicfaace

@epicfaace epicfaace commented Jan 6, 2019

Copy link
Copy Markdown
Contributor

@the-knights-who-say-ni

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!

@tirkarthi

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.

@epicfaace

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.

@epicfaace epicfaace changed the title bpo-12707: deprecate info(), geturl(), getcode() methods in favor of info, url, and status properties for HTTPResponse and addinfourl Jan 7, 2019
@merwok merwok self-requested a review January 7, 2019 02:22
1 hidden conversation Load more…
@bedevere-bot

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.

67 hidden items Load more…
@csabella csabella reopened this May 22, 2019
@epicfaace

Copy link
Copy Markdown
Contributor Author

@csabella @merwok any updates on this?

@matrixise matrixise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

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

@bedevere-bot

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!

@epicfaace

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

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

matrixise
matrixise previously approved these changes Sep 13, 2019

@matrixise matrixise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Thank you for your contribution

@matrixise

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.

@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
@matrixise

Copy link
Copy Markdown
Member

Thank you so much for your contribution, I closed the issue and have merged this PR.

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants