Bug 736994 - Review request: rubygem-bson_ext - C extensions for Ruby BSON
Summary: Review request: rubygem-bson_ext - C extensions for Ruby BSON
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: 705531
TreeView+ depends on / blocked
Reported: 2011-09-09 09:51 UTC by Bohuslav "Slavek" Kabrda
Modified: 2012-04-02 13:12 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2011-09-26 08:10:49 UTC
vondruch: fedora-review+
gwync: fedora-cvs+

Attachments (Terms of Use)

Description Bohuslav "Slavek" Kabrda 2011-09-09 09:51:03 UTC
SPEC URL: http://bkabrda.fedorapeople.org/bson_ext/rubygem-bson_ext.spec
SRPM URL: http://bkabrda.fedorapeople.org/bson_ext/rubygem-bson_ext-1.3.1-1.fc15.src.rpm

Description: C extensions to accelerate the Ruby BSON serialization. For more information about BSON, see http://bsonspec.org.  For information about MongoDB, see http://www.mongodb.org.

Comment 1 Vít Ondruch 2011-09-09 09:55:30 UTC
I'll take this one.

Comment 2 Vít Ondruch 2011-09-09 12:08:59 UTC
* Descriptive filenames
  - It would be fine if the Source1 and Source2 files could have more descriptive
    names, such as %{gemname}-%{version}-tests-upstream.tgz and 

* Make sure that the test suite is really using the C extension
  - There is recorded following notice in buildlog, which makes me wonder if the
    test suite is really doing what is supposed to do:

    "Notice: C extension not loaded. This is required for optimum MongoDB Ruby
     driver performance."

* pushd should be paired with popd
  - It is considered good habit I would say

* Test cleanup
  - The test suite is extracted in temporary directory, but it is not later

* Files not required for runtime should be moved into -doc subpackage
  - Please move "%{geminstdir}/Rakefile" and "%{geminstdir}/bson_ext.gemspec" 
    into -doc subpackage according to [1]

[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Documentation

Comment 3 Bohuslav "Slavek" Kabrda 2011-09-09 12:40:22 UTC
There seems to be an issue with tests on i386 architecture:

  1) Error:
RangeError: bignum too big to convert into `long'
    ./test/bson/object_id_test.rb:137:in `test_from_time_unique'

It seems that the long on i386 is not long enough for the particular used
bignum, since on x86_64 everything works fine.

I reported the bug here:

Comment 4 Bohuslav "Slavek" Kabrda 2011-09-23 05:45:02 UTC
I've fixed the issues and updated to the last upstream version, which solved the problem mentioned in Comment 3.
SPEC: http://bkabrda.fedorapeople.org/bson_ext/rubygem-bson_ext.spec
SRPM: http://bkabrda.fedorapeople.org/bson_ext/rubygem-bson_ext-1.4.0-1.fc15.src.rpm

Comment 5 Vít Ondruch 2011-09-23 08:46:28 UTC
* Incoherent version in changelog
  - From rpmlint outpu: rubygem-bson_ext.i686: W: incoherent-version-in-changelog
    1.3.1-3 ['1.4.0-1.fc17', '1.4.0-1']
  - You forgot to update the changelog version accordingly.

* The test cleanup should be done in the same section as the test preparation
  - Since the test are unpacked required just in %check section, I would suggest
    to clean them in the same section
  - Another alternative would be to unpack the using %setup macro, but I am not
    aware of any other package which is prepared this way. It might be worth of

Please fix this nits before import. Otherwise the package looks good => APPROVED

Comment 6 Bohuslav "Slavek" Kabrda 2011-09-23 10:41:44 UTC
New Package SCM Request
Package Name: rubygem-bson_ext
Short Description: C extensions for Ruby BSON
Owners: vondruch

Comment 7 Gwyn Ciesla 2011-09-24 15:50:08 UTC
Git done (by process-git-requests).

Comment 8 Vít Ondruch 2012-04-02 13:08:51 UTC
Package Change Request
Package Name: rubygem-bson_ext
New Branches: f16
Owners: vondruch jknife

Comment 9 Gwyn Ciesla 2012-04-02 13:12:19 UTC
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.