Bug 828278
| Summary: | Review Request: rubygem-sinatra-rabbit - Ruby DSL for creating restful applications using Sinatra | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Michal Fojtik <mfojtik> |
| Component: | Package Review | Assignee: | Vít Ondruch <vondruch> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | notting, package-review, vondruch |
| Target Milestone: | --- | Flags: | vondruch:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2012-06-26 00:30:24 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Michal Fojtik
2012-06-04 13:43:42 UTC
* Mon Jun 04 2012 Michal Fojtik <mfojtik> 1.0.6-2 - Added exclude tag before gem_cache - Removed the ruby dependency Spec URL: http://omicron.mifo.sk/rpms/rubygem-sinatra-rabbit.spec SRPM URL: http://omicron.mifo.sk/rpms/rubygem-sinatra-rabbit-1.0.6-2.fc17.src.rpm I'll take it for a review. * 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
Thanks Vit! Since it's a Sinatra extension, you need to have 'sinatra/base' required first before you require this extension. Probably a good point to add some additional treating for errors like this. I'll fix all things above. New Package SCM Request ======================= Package Name: rubygem-sinatra-rabbit Short Description: Ruby DSL for creating restful applications using Sinatra Owners: mfojtik Branches: f16 f17 el6 InitialCC: mfojtik Git done (by process-git-requests). rubygem-sinatra-rabbit-1.0.6-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/rubygem-sinatra-rabbit-1.0.6-2.fc16 rubygem-sinatra-rabbit-1.0.6-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/rubygem-sinatra-rabbit-1.0.6-2.fc17 rubygem-sinatra-rabbit-1.0.6-2.fc16 has been pushed to the Fedora 16 testing repository. rubygem-sinatra-rabbit-1.0.6-2.fc17 has been pushed to the Fedora 17 stable repository. rubygem-sinatra-rabbit-1.0.6-2.fc16 has been pushed to the Fedora 16 stable repository. |