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

Align json option with request, improved to include input from #134 comments #139

Merged
merged 8 commits into from
Nov 20, 2016

Conversation

naugtur
Copy link
Owner

@naugtur naugtur commented Nov 12, 2016

I took #135 and updated it to become a 99% backward compatible change.

The only change in behavior from previous version is not sending "true" as json body, which was incorrect anyway.

I believe we could even release this as a minor version update.
It depends if we consider it a breaking change or a bugfix that only affects a case of invalid JSON.

Thoughts?

@naugtur
Copy link
Owner Author

naugtur commented Nov 12, 2016

BTW. This change wouldn't be possible with everyone's inputs.

Big thanks to @bjoerge, @wesleytodd, @TehShrike, @mhart for enabling it!

@naugtur naugtur changed the title Bjoerge align json option with request improved to include input from #134 comments Align json option with request, improved to include input from #134 comments Nov 12, 2016
@wesleytodd
Copy link

Looks good to me! LGTM

@djake
Copy link
Contributor

djake commented Nov 18, 2016

@naugtur Do you anticipate merging this in soon? It would be great to have.

@naugtur
Copy link
Owner Author

naugtur commented Nov 20, 2016

I hoped for a discussion, but I'm certain this is a good change.

@naugtur naugtur merged commit b7f760e into master Nov 20, 2016
@wesleytodd
Copy link

Agreed, I am unsure what else there would be to discuss. This was released in 2.3.0, correct?

@naugtur
Copy link
Owner Author

naugtur commented Nov 22, 2016

Correct.

@dhritzkiv dhritzkiv mentioned this pull request Nov 23, 2016
5 tasks
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.

4 participants