Skip to content

Fix for #1728#1729

Closed
gazpachoking wants to merge 2 commits into
psf:masterfrom
gazpachoking:#1728
Closed

Fix for #1728#1729
gazpachoking wants to merge 2 commits into
psf:masterfrom
gazpachoking:#1728

Conversation

@gazpachoking

Copy link
Copy Markdown
Contributor

This is how I think things should be fixed if we are okay with adding the cookiejar to PreparedRequests. What do you guys think?

@Lukasa

Lukasa commented Nov 7, 2013

Copy link
Copy Markdown
Member

I want to have a discussion on #1728 before we merge this. =)

I'll make some comments inline on the diff anyway though.

Comment thread requests/models.py

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.

I want to hide this from people. I think if we're doing this we should treat it as an implementation detail, e.g. this becomes _cookies.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 for hiding this

@Lukasa Lukasa mentioned this pull request Nov 28, 2013
2 tasks
@sigmavirus24

Copy link
Copy Markdown
Contributor

@Lukasa if I take over the rest of this PR (to address our feedback) would we be comfortable getting it into 2.1? I feel like this addresses a real concern and I think you were in agreement with @gazpachoking at the end of #1728. Correct me if I'm wrong.

For reference, I'll pull his commits onto a branch of mine, and just fix up the one minor thing and send a new PR and then close this one. Sound good?

@Lukasa

Lukasa commented Dec 4, 2013

Copy link
Copy Markdown
Member

If you think you can get it done in a fairly swift manner, sure. =)

sigmavirus24 added a commit to sigmavirus24/requests that referenced this pull request Dec 4, 2013
- Make the PreparedRequest's cookie jar an implementation detail
@sigmavirus24

Copy link
Copy Markdown
Contributor

@Lukasa done in #1776

kennethreitz added a commit that referenced this pull request Dec 5, 2013
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants