[Bug 616779] Review Request: rubygem-json_pure - JSON implementation in pure Ruby

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=616779

--- Comment #5 from Michal Fojtik <mfojtik at redhat.com> 2010-08-02 04:20:20 EDT ---
(In reply to comment #4)
> Well, the problem is that
> - rubygem-json already installs %{_bindir}/edit_json, %{_bindir}/prettify_json.
>   So installing both rubygem-json, rubygem-json_pure may easily makes
>   rpm conflict.
> - if ruby -rubygems -e "require 'json'" is to be executed, we are not
>   sure if we are using pure json ext/ or C module json ext/

Ah ok, I see. There might be solution in prefixing these two binaries with
'_pure'. But I'm not sure if there will not be another conflicts.
So I'll try updating cucumber with just 'json' and we will see.

> 
> By the way
> (In reply to comment #3)
> > Cucumber and other projects uses this because it's a "little bit" faster that
> > original one.    
> - Is it true that "pure" implementation is faster than C extension?

According to [1], 'pure' parser seems to be faster:

[snip]
#   1 ParserBenchmarkExt#parser   900 repeats:
#         553.922304770 (  real) ->   21.500x 
#           0.001805307
#   3 ParserBenchmarkPure#parser  1000 repeats:
#          26.755020642 (  real) ->    1.038x 
#           0.037376163
[/snip]

(but I'm not sure if my undestanding of these benchmarks is right.

> For spec file:
> - For Fedora, BuildRoot tag is no longer needed (only needed for EPEL5)

Fixed.

> - If the license is "the same as Ruby", the license tag should be
>   "GPLv2 or Ruby".

Fixed.

> - edit_json ahd prettify_json have the dependency for ruby(gtk2),
>   so this package should have "Requires: ruby(gtk2)".
>   For this issue, see the discussion on bug 589801

Fixed.

> - "install.rb" "ext/" files or directories are not needed
>   * ext/ directory is for generating C extension modules. For
>     pure implementation, this directory should not be used.

Removed.

>     By the way:
> (In reply to comment #1)
> > Binary objects under tests directory are removed (%check), because they
> > are only needed for test as an generators. 
>     - I think Rakefile should be patched so that "$ rake check" also uses
>       pure "generator" (lib/json/pure/generator.rb) instead of
>       C extension generator.so (i.e so that ext/ directory is not
>       needed even for rake check, if this gem is for pure implementation,
>       and we want to check pure implementation on test)

Fixed. Rakefile already contains 'rake test_pure' which will use 'pure'
generator.

> - "benchmarks/" "tools/" "data/" "tests/" directories does not seem
>   to be needed by default and it seems they are just documents,
>   examples or so.
>   Please mark these as %doc, or instead moving these directories
>   (and ri document files) to -doc subpackage is preferred.

Fixed. (Moved to -docs subpackage)

> - defined %ruby_sitelib macro seems used nowhere.

Fixed.

> - The directory %{geminstdir} is not owned by any packages.    

Fixed.

rev -2:

Spec URL: http://mifo.sk/RPMS/rubygem-json_pure.spec
SRPM URL: http://mifo.sk/RPMS/rubygem-json_pure-1.4.3-2.fc13.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.