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=642583 --- Comment #12 from Vít Ondruch <vondruch@xxxxxxxxxx> 2012-01-23 10:52:50 EST --- (In reply to comment #11) > I would like to pick this one if there's no objection. I spoke to Michal and he seems to be fine with you finishing the review. So I'll do the review of your .spec. * ruby_sitearch - This macro is not used in your spec, please remove it. * Summary is not descriptive - It would be nice if you can extend the summary a bit. * Test suite - Usage of Rake just brings in unnecessary dependencies. Please do not use rake. - Please do not use Bundler for running test suite. It usually requires gems which are not really necessary (for example session appears to be one of them) and versions not available in Fedora (for example ruby-debug). In the end, it is easier to remove the Bundler dependency: sed -i -e '/require "bundler"/d' spec/environment_fixture_setup.rb sed -i -e '/require "bundler"/d' spec/spec_suite.rb - Use RSpec 2.x. I'd like to see RSpec 1.x to die and it would be shame to introduce newly reviewed gem which depends on RSpec 1.x. Unfortunately, the test suite is not yet working with RSpec 2.x, although it seems there is already pull request [1] to add this support. * Do not mark spec and benchmarks as a %doc - The spec and benchamrks are not documentation, so please remove the %doc macro. * Exclude the cached version of gem - In RPM based system, there is no need to cache the .gem file. Please consider its removal. BTW, since I hope Ruby 1.9.3 will be today approved by FESCo, have you considered to provide .spec file for Ruby 1.9? [1] https://github.com/btakita/rr/pull/68 -- 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