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

2.54: Support Cpanel::JSON::XS as other fast XS choice #8

Closed
wants to merge 2 commits into from

Conversation

rurban
Copy link

@rurban rurban commented Apr 4, 2013

Support Cpanel::JSON::XS as other fast JSON XS choice.
Cpanel::JSON::XS has a public bugtracker,
is maintained by cPanel Inc,
works also with 5.6.2
does not use common::sense at run-time, only during testing.

Also fix some deprecated names in the testsuite.
Skip non-matching prototype warnings.

Reini Urban added 2 commits April 4, 2013 18:46
Support Cpanel::JSON::XS as other fast XS choice.
Cpanel::JSON::XS has a public bugtracker,
is maintained by cPanel Inc,
works also with 5.6.2
does not use common::sense at run-time, only during testing.

Also fix some deprecated names in the testsuite.
Skip non-matching prototype warnings.
@rurban
Copy link
Author

rurban commented Apr 6, 2013

Had to change to to,from_json prototypes from $@ to $;$ to match the XS version and avoid prototype mismatch warnings.

@schwern
Copy link

schwern commented Apr 23, 2013

Seconded. I would like to be able to use a fast JSON parser with a compatible interface without risking myself or my users interacting with MLEHMANN.

@rurban
Copy link
Author

rurban commented Apr 23, 2013

Thanks. We had some cPanel internal discussion, and we will rename the
module to
cPanel::JSON::XS and upload it on the CPANEL account to CPAN.
There needs also some possible 5.6 utf8 issues to be sorted out first.
Please wait until then.

On Tue, Apr 23, 2013 at 3:28 PM, Michael G. Schwern <
notifications@github.com> wrote:

Seconded. I would like to be able to use a fast JSON parser with a
compatible interface without risking myself or my users interacting with
MLEHMANN.


Reply to this email directly or view it on GitHubhttps://github.com//pull/8#issuecomment-16884221
.

Reini Urban
http://cpanel.net/ http://www.perl-compiler.org/

@rurban
Copy link
Author

rurban commented May 22, 2013

We sorted it out. Unfortunately the name has to stay as it is, even we dont like it. I already uploaded that name.
mst also uploaded now his JSON chooser JSON::MaybeXS using our name Cpanel::JSON::XS and not JSON::XS.
It can be merged as is.

@tokuhirom
Copy link

I don't agree to merge this patch. The main issue around JSON::XS was resolved.
(I guess the main issue of the JSON::XS at Apr. 2013 is mlehmann does not fix the issue around hash randomization)

@makamaka
Copy link
Owner

makamaka commented Aug 1, 2013

I don't mind supporting Cpanel::JSON::XS. (though the turn to check is JSON::XS first)

The reasons:

  • C::J::X supports Perl 5.6.
  • that addition hardly affect others.

What do you think about this, tokuhirom-san?

@tokuhirom
Copy link

Perl 5.6 support was dropped by toolchain. ref. https://github.com/Perl-Toolchain-Gang/toolchain-site/blob/master/lancaster-consensus.md
I think there is no reason to support 5.6 in 2013 by JSON.pm.

@tokuhirom
Copy link

The modules trying to load installed modules are terrible. It makes huge bugs.
Remember JSON::Any did make terrible issues.

@makamaka
Copy link
Owner

makamaka commented Aug 1, 2013

@makamaka makamaka closed this Aug 1, 2013
@Songmu
Copy link

Songmu commented Aug 1, 2013

I write up my opinion summary. (I know this issue is already closed.)

The format of JSON, JSON::PP and JSON::XS are simple and clear.

I don't think JSON needs multi XS backend support.
it mess up more complexity in code and is confusing.

XS backend is should be limited only one module.

JSON::XS may break temporary and Cpanel::JSON::XS also may break temporary too.

It it that, if JSON supports multi XS backend,
Many CPAN modules depending on JSON cost more maintainaunce fee.

Support multi XS backend may cause someone feels convinience, but it is not good
at JSON.pm which should be stable.

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.

None yet

5 participants