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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Vít Ondruch <vondruch@xxxxxxxxxx> changed:

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

--- Comment #3 from Vít Ondruch <vondruch@xxxxxxxxxx> ---
* 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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]