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

Drop deprecates #3071

Closed
wants to merge 3 commits into from
Closed

Drop deprecates #3071

wants to merge 3 commits into from

Conversation

T-256
Copy link
Contributor

@T-256 T-256 commented Jan 25, 2024

Summary

version 1.0 proposal
Resolves requirements for #3069 (review)

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@T-256 T-256 marked this pull request as draft January 25, 2024 11:22
@T-256 T-256 force-pushed the deprecations branch 3 times, most recently from 9ae5b3b to 4e67f08 Compare January 25, 2024 12:16
@T-256 T-256 force-pushed the deprecations branch 2 times, most recently from fb8805a to 3dcc429 Compare January 25, 2024 12:57
httpx/_client.py Outdated Show resolved Hide resolved
tests/client/test_proxies.py Show resolved Hide resolved
@tomchristie
Copy link
Member

Great. Needs a CHANGELOG.md entry.
Once we've got that I think we're good to merge this.

@T-256
Copy link
Contributor Author

T-256 commented Jan 25, 2024

The purpose of this is for v1.0
If next release is not 1.0 then I'd recommend don't merge this soon.

I thought #3069 will include changelogs. For example for this PR:

Removed

  • proxies parameter from client.
  • data parameter no longer accepts non mapping objects.
  • cookies parameter no longer accepts per requests.

@tomchristie Do any docs need to be updated?

@tomchristie
Copy link
Member

The purpose of this is for v1.0

My feeling is that we should push on with 1.0.

(Tho I also think that this particular change would be acceptable even in 0.27 release, since it's only removing features that are already formally deprecated.)

@tomchristie
Copy link
Member

Neat.

Do any docs need to be updated?

For this PR just the CHANGELOG.md.

Comment on lines +16 to +18
* Recently deprecated `proxies` argument has been removed. (#3071)
* Per request `cookies` argument is not supporting anymore. (#3071)
* Non-Mapping objects for `data` argument is no longer allowed. (#3071)
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest we limit the pull request to just one line item here, and deal with the proxies case.

@rafalkrupinski
Copy link

Please don't remove per-request cookies :)
#2931 - proposition to set domain from the request URL when cookies are passed as a dictionary.

@tomchristie
Copy link
Member

Targeting just the proxies=... change is the thing to do here.
Let's keep PRs sharply targeted. 🎯

@T-256
Copy link
Contributor Author

T-256 commented Feb 2, 2024

I ended up to not drop any public api before major release.
So, closing this for now.

@T-256 T-256 closed this Feb 2, 2024
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.

3 participants