[Bug 642583] Review Request: rubygem-rr - RR is a framework that features a rich selection of double techniques

[bugzilla at redhat.com]

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 at redhat.com> 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.