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=810926 Vít Ondruch <vondruch@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |vondruch@xxxxxxxxxx Flag| |fedora-review? --- Comment #12 from Vít Ondruch <vondruch@xxxxxxxxxx> 2012-04-24 03:12:00 EDT --- Ok, going to finish what I was already started. Taking the review. (In reply to comment #11) > I do have one question: we're likely going to rename the gem from qpid to > qpid-messaging due to the owner of the original qpid gem on RubyForge.org not > following through with his promise to release that name to our project. How > will this affect the review process? Well, during review, it is easy process, you just need to udpate the .spec file ant the ticket subject and that is it. However, I would prefer to see the gem with the current name on rubygems.org (I know, I know, I'm silent ;)) * License - The LICENSE file contains Apache license and Boost license. I am wondering what is purpose of the Boost license? Shouldn't it mentioned in license field? Or removed from the LICENSE file? * Test suite - I'd love to see test suite executed in %check section. * Binary extension placement. - Please move the .so file into the correct placement according to the guidelines: # If there are C extensions, mv them to the extdir. # You should replace REQUIRE_PATHS with the first value of the require_paths field in # the gemspec file. It will typically be either "lib" or "ext". For instance: # s.require_paths = ["lib"] mkdir -p %{buildroot}%{gem_extdir}/REQUIRE_PATHS mv %{buildroot}%{gem_instdir}/REQUIRE_PATHS/shared_object.so %{buildroot}%{gem_extdir}/REQUIRE_PATHS/ * Use macros provided by rubygems-devel - There is more macros provided by rubygems-devel, such as %{gem_libdir}, %{gem_cache}, %{gem_spec}, %{gem_docdir} and %{gem_extdir}. It would be nice to use them in %files section and also elsewhere where suitable. * Move some unnecessary files into -doc subpackage - the following files/dirs are not mandatory for runtime and should be moved into the -doc subpackage: %{gem_instdir}/examples %{gem_instdir}/features %{gem_instdir}/spec %{gem_instdir}/Rakefile %doc %{gem_instdir}/TODO * The package does not work. - I tried the most trivial test case, which unfortunately failed: # ruby -e 'require "qpid"' /usr/share/rubygems/rubygems/custom_require.rb:36:in `require': /usr/share/gems/gems/qpid-0.16.0/lib/qpid/encoding.rb:48: syntax error, unexpected ':', expecting keyword_then or ',' or ';' or '\n' (SyntaxError) when "amqp/map": Cqpid.decodeMap message.message_impl ^ /usr/share/gems/gems/qpid-0.16.0/lib/qpid/encoding.rb:49: syntax error, unexpected keyword_when, expecting keyword_end when "amqp/list": Cqpid.decodeList message.message_impl ^ /usr/share/gems/gems/qpid-0.16.0/lib/qpid/encoding.rb:49: syntax error, unexpected ':', expecting keyword_end when "amqp/list": Cqpid.decodeList message.message_impl ^ /usr/share/gems/gems/qpid-0.16.0/lib/qpid/encoding.rb:57: syntax error, unexpected keyword_end, expecting $end from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require' from /usr/share/gems/gems/qpid-0.16.0/lib/qpid.rb:23:in `<top (required)>' from /usr/share/rubygems/rubygems/custom_require.rb:60:in `require' from /usr/share/rubygems/rubygems/custom_require.rb:60:in `rescue in require' from /usr/share/rubygems/rubygems/custom_require.rb:35:in `require' from -e:1:in `<main>' -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review