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

Json::encode - pass all options to json_encode #89

Closed
wants to merge 1 commit into from

Conversation

@ondrejmirtes
Copy link
Contributor

ondrejmirtes commented Oct 16, 2015

Main motivation is to be able to use JSON_PRESERVE_ZERO_FRACTION while still benefitting from other features of Json::encode like error detection.

There are no intentional BC breaks involved.

@ondrejmirtes ondrejmirtes force-pushed the ondrejmirtes:json-encode-options branch from 1430690 to acccbbe Oct 16, 2015
@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Oct 16, 2015

JSON_PRESERVE_ZERO_FRACTION should be default behavior

@ondrejmirtes

This comment has been minimized.

Copy link
Contributor Author

ondrejmirtes commented Oct 16, 2015

But that would be a big BC break. So unless we want to release 3.0 the default behaviour should stay the same.

@ondrejmirtes ondrejmirtes force-pushed the ondrejmirtes:json-encode-options branch from acccbbe to 05a3b32 Oct 16, 2015
@dg

This comment has been minimized.

Copy link
Member

dg commented Oct 19, 2015

To pass all options goes against the spirit of this class.

I agree that JSON_PRESERVE_ZERO_FRACTION should be default in 3.0 (due to compatibility and due to minimal required PHP version) because stripping zero fraction is bug in fact.

It is not BC break when you export from PHP to JavaScript, but it is BC break when you export from PHP to PHP. The question is whether it's a problem or bugfix.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Oct 19, 2015

I see JSON_PRESERVE_ZERO_FRACTION as bugfix. The previous behavior makes not sense. Why would anyone expect and rely on json_decode(json_encode(2.0)) === 2?

@ondrejmirtes

This comment has been minimized.

Copy link
Contributor Author

ondrejmirtes commented Oct 20, 2015

It might make no sense, but it's the current behaviour and you can't make any assumptions whether applications rely on it or not.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Oct 20, 2015

What? Of course you can make assumptions. Otherwise you will never change anything cause everything change is BC break.

@ondrejmirtes

This comment has been minimized.

Copy link
Contributor Author

ondrejmirtes commented Oct 20, 2015

It's OK to do BC breaks but not in a patch or minor version.

@dg dg closed this in edab9cd Nov 29, 2015
dg added a commit that referenced this pull request Dec 3, 2015
dg added a commit that referenced this pull request Dec 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.