[RFC] [VOTE] Preserve Fractional Part in JSON encode

9 years ago by Juan Basso — view source

unread

I'd like to initiate a vote on this RFC:
https://wiki.php.net/rfc/json_preserve_fractional_part

TLDR: this RFC would add a new json_encode option to preserve fractional
part of float numbers when they are 0. Ie, encoding (float)10 to json will
be 10.0.
PS: This is not enabled by default.

The code was implemented and available on
https://github.com/php/php-src/pull/642

Preview discussions on:

Thanks,
Juan Basso

9 years ago by Pascal MARTIN — view source

unread

I'd like to initiate a vote on this RFC:
https://wiki.php.net/rfc/json_preserve_fractional_part

Hi,

After discussing this RFC with other people of AFUP, we are +1 on this.

Being able to get back the same PHP type after encoding data to JSON and
re-decoding it could be useful (thinking about strict comparison,
typically), even if JSON only has one "number" type.

And adding a new flag doesn't have much negative impact (as long as it's
not enabled by default, especially for a minor version of PHP).

--
Pascal MARTIN, AFUP - French UG
http://php-internals.afup.org/

9 years ago by Juan Basso — view source

unread

The vote is now closed. By 14x0 the RFC was accepted.

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was target to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

Thanks.

On Sat, Jan 17, 2015 at 4:48 PM, Pascal MARTIN, AFUP <
mailing@pascal-martin.fr> wrote:

I'd like to initiate a vote on this RFC:
https://wiki.php.net/rfc/json_preserve_fractional_part

Hi,

After discussing this RFC with other people of AFUP, we are +1 on this.

Being able to get back the same PHP type after encoding data to JSON and
re-decoding it could be useful (thinking about strict comparison,
typically), even if JSON only has one "number" type.

And adding a new flag doesn't have much negative impact (as long as it's
not enabled by default, especially for a minor version of PHP).

--
Pascal MARTIN, AFUP - French UG
http://php-internals.afup.org/

9 years ago by Pierre Joye — view source

unread

The vote is now closed. By 14x0 the RFC was accepted.

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was target
to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

I miss that rfc and especially the part saying it targets next 5.x.y, aka
next 5.6.

This is not good. We do not allow features addition to stable branch.

Cheers,
Pierre

9 years ago by Juan Basso — view source

unread

During the discussion process it was discussed and people were ok with it
since it doesn't break any existing code. Also, JSON_NUMERIC_CHECK option
was added on json_encode in patch version (added on 5.3.3).

The vote is now closed. By 14x0 the RFC was accepted.

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was
target to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

I miss that rfc and especially the part saying it targets next 5.x.y, aka
next 5.6.

This is not good. We do not allow features addition to stable branch.

Cheers,
Pierre

9 years ago by Pierre Joye — view source

unread

During the discussion process it was discussed and people were ok with it
since it doesn't break any existing code. Also, JSON_NUMERIC_CHECK option
was added on json_encode in patch version (added on 5.3.3).

I do not know who are the "people" but the release process is pretty
clear on that. We decided not to allow features additions to ease
developers life, like not having to do dozen of tests to know which
minor versions added what. It is also why we have yearly releases,
reduce the delay between accepted features and actual availability in
a release.

Please not that the release process was approved after 5.3 and applied to 5.4+.

--
Pierre

@pierrejoye | http://www.libgd.org

9 years ago by Nikita Popov — view source

unread

During the discussion process it was discussed and people were ok with it
since it doesn't break any existing code. Also, JSON_NUMERIC_CHECK option
was added on json_encode in patch version (added on 5.3.3).

I do not know who are the "people" but the release process is pretty
clear on that. We decided not to allow features additions to ease
developers life, like not having to do dozen of tests to know which
minor versions added what. It is also why we have yearly releases,
reduce the delay between accepted features and actual availability in
a release.

Please not that the release process was approved after 5.3 and applied to
5.4+.

The release process RFC explicitly allows the addition of self-contained
features in patch releases. This rule is implemented in practice either by
RM approval for the feature or an RFC, the latter being the case here.

Nikita

9 years ago by Pierre Joye — view source

unread

During the discussion process it was discussed and people were ok with
it
since it doesn't break any existing code. Also, JSON_NUMERIC_CHECK
option
was added on json_encode in patch version (added on 5.3.3).

I do not know who are the "people" but the release process is pretty
clear on that. We decided not to allow features additions to ease
developers life, like not having to do dozen of tests to know which
minor versions added what. It is also why we have yearly releases,
reduce the delay between accepted features and actual availability in
a release.

Please not that the release process was approved after 5.3 and applied
to 5.4+.

The release process RFC explicitly allows the addition of
self-contained features in patch releases. This rule is implemented in
practice either by RM approval for the feature or an RFC, the latter being
the case here.

Except that this one is not self contained and the rfc says in rare cases
(or say, for good reason).

I give up to try explain where the constant lack of respect of the RFC will
lead. That's why I won't battle for this now.

9 years ago by Andrea Faulds — view source

unread

Hey Pierre,

Except that this one is not self contained and the rfc says in rare cases
(or say, for good reason).

Does it? With the flag turned off (default), behaviour is exactly the same as before.

With the flag on, behaviour is changed.

Seems fairly self-contained to me, just an extra flag. It does affect performance, but positively, so not a problem.

Thanks.

--
Andrea Faulds
http://ajf.me/

9 years ago by Stanislav Malyshev — view source

unread

Hi!

I do not know who are the "people" but the release process is pretty
clear on that. We decided not to allow features additions to ease
developers life, like not having to do dozen of tests to know which
minor versions added what. It is also why we have yearly releases,

We have always added small features in patch versions. Just look in the
UPGRADING or NEWS. E.g:

5.6.5:
. Fixed bug #68479 (Added escape parameter to SplFileObject::fputcsv).
(Salathe)

5.6.3:
. Implemented gmp_random_range() and gmp_random_bits(). (Leigh)

5.6.1:
. Implemented gmp_import() and gmp_export(). (Leigh, Nikita)

I'm pretty sure there is more in 5.5. I don't see how suddenly it is not
allowed. This is an option addition, something that always was OK in
patch versions. Or do you see some other concern that I miss here?

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Ferenc Kovacs — view source

unread

The vote is now closed. By 14x0 the RFC was accepted.

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was target
to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

I miss that rfc and especially the part saying it targets next 5.x.y, aka
next 5.6.

This is not good. We do not allow features addition to stable branch.

Cheers,
Pierre

https://wiki.php.net/rfc/releaseprocess
"No feature addition after final x.y.0 release (or x.0.0). Self contained
features or new SAPIs could be carefully considered on a case by case
basis."
This is a small, self-contained change, and we just had a vote for satisfy
the case-by-case basis. What am I missing here?

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

9 years ago by Stanislav Malyshev — view source

unread

Hi!

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was target to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

No, I think we can do with the existing one. I'll merge it soon if there
aren't more objections.

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Juan Basso — view source

unread

Can you also resolve https://bugs.php.net/bug.php?id=50224 ?

Thanks.

Hi!

Since I am not part of the core team, who can merge the PR (
https://github.com/php/php-src/pull/642)? By the way, the RFC was
target to
the next PHP 5.6 patch version, but the PR was target to master. Should I
reopen the PR to PHP-5.6 branch?

No, I think we can do with the existing one. I'll merge it soon if there
aren't more objections.

--
Stas Malyshev
smalyshev@gmail.com