Everything

php

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

php.internals

Juan Basso

11 years ago

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:
- http://marc.info/?l=php-internals&m=141507087629656&w=2
- http://marc.info/?l=php-internals&m=141953602116283&w=2

Thanks,
Juan Basso

Pascal MARTIN

11 years ago

On 11/01/2015 04:02, Juan Basso 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/

Juan Basso

11 years ago

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:

Pierre Joye

11 years ago

On Jan 18, 2015 11:24 AM, "Juan Basso" <jrbasso@gmail.com> wrote:

>
> 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

Juan Basso

11 years ago

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).

On Sun, Jan 18, 2015 at 12:37 AM, Pierre Joye <pierre.php@gmail.com> wrote:

Pierre Joye

11 years ago

On Sun, Jan 18, 2015 at 11:27 PM, Juan Basso <jrbasso@gmail.com> wrote:

> 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

Nikita Popov

11 years ago

On Sun, Jan 18, 2015 at 6:02 PM, Pierre Joye <pierre.php@gmail.com> wrote:

> On Sun, Jan 18, 2015 at 11:27 PM, Juan Basso <jrbasso@gmail.com> wrote:
> > 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

Pierre Joye

11 years ago

On Jan 19, 2015 12:13 AM, "Nikita Popov" <nikita.ppv@gmail.com> wrote:

>
> On Sun, Jan 18, 2015 at 6:02 PM, Pierre Joye <pierre.php@gmail.com> wrote:
>>
>> On Sun, Jan 18, 2015 at 11:27 PM, Juan Basso <jrbasso@gmail.com> wrote:
>> > 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.

Andrew Faulds

11 years ago

Hey Pierre,

> On 18 Jan 2015, at 17:17, Pierre Joye <pierre.php@gmail.com> wrote:
>
> 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/

Stas Malyshev

11 years ago

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

Ferenc Kovacs

11 years ago

On Sun, Jan 18, 2015 at 6:37 AM, Pierre Joye <pierre.php@gmail.com> wrote:

> On Jan 18, 2015 11:24 AM, "Juan Basso" <jrbasso@gmail.com> wrote:
> >
> > 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

Stas Malyshev

11 years ago

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

Juan Basso

11 years ago

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

Thanks.
On Jan 19, 2015 3:48 AM, "Stanislav Malyshev" <smalyshev@gmail.com> wrote: