[Bug 828278] Review Request: rubygem-sinatra-rabbit - Ruby DSL for creating restful applications using Sinatra

bugzilla at redhat.com bugzilla at redhat.com
Tue Jun 5 08:39:23 UTC 2012


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

Vít Ondruch <vondruch at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?

--- Comment #3 from Vít Ondruch <vondruch at redhat.com> ---
* Please consider running test in %{_builddir}
  - Could you consider to run test suite in %{_builddir} instead of
%{buildroot}?
    I.e. use "pushd .%{gem_instdir}" in %check section. In my experience, it
may
    prevent pollution of the resulting package by some temporary dirs/files.

* Use %exclude instead of rm
  - You might consider to use %exclude in files section instead of rm in
%install
    section. This is just a hint.

* I am not expert on Sinatra, but this doesn't look good:

# irb
irb(main):001:0> require 'sinatra/rabbit'
NameError: uninitialized constant Sinatra
    from /usr/share/gems/gems/sinatra-rabbit-1.0.6/lib/sinatra/rabbit.rb:33: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 (irb):1
    from /usr/bin/irb:12:in `<main>'

  Nevertheless, this is probably not a real life problem, so it is not blocking
  the review.


Overall, the package looks good and all the issues mentioned above are just
minor nits => APPROVED

-- 
You are receiving this mail because:
You are on the CC list for the bug.


More information about the package-review mailing list