RPMLINT OUTPUT:

$ rpmlint /home/quake/rpmbuild/SRPMS/rubygem-compass-rails-1.0.3-1.fc18.src.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-compass-rails-1.0.3-1.fc18.noarch.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-compass-rails-doc-1.0.3-1.fc18.noarch.rpm rubygem-compass-rails.spec
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

I'm going to review this.

* Wrong license
  - The LICENSE file states MIT. Please fix the license

* Remove "Requires: ruby"
  - Since we are going to have better integration with JRuby in F19, this would
    force the dependency on MRI, while it should work with JRuby as well.

* compass-rails is incompatible with compass in Fedora
  - Testing with F19:

    $ cd compass-rails-1.0.3/
    $ ruby -Ilib -e 'require "compass-rails"'
    /compass-rails-1.0.3/lib/compass-rails.rb:228:in `<top (required)>':
            undefined method `register' for Compass::AppIntegration:Module
            (NoMethodError)
	from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
	from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
	from -e:1:in `<main>'

* Test suite does not passes
  - It is executed, but 0 tests is executed, since they fail on missing
    dependencies. You need to add "BuildRequires: rubygem(compass)" at least,
    but then you'll be bitten by the incompatibility with rubygem-compass
    mentioned above.
  - I would suggest to use rubygem(minitest) for testing instead of
    rubygem(test-unit), if possible.
  - It seems that the tests depends on Bundler. It will be needed to get rid of
    it I guess.

* Move file not needed for runtime into -doc subpackage
  - Following files are not needed by runtime IMO

    %{gem_instdir}/Appraisals
    %{gem_instdir}/Gemfile
    %{gem_instdir}/Guardfile

    Please consider to move them into -doc subpackage.
  - I would move the %doc %{gem_instdir}/README.md into -doc as well, but I'll
    keep it up to you.

Spec URL: http://tdawson.fedorapeople.org/rubygems/rubygem-compass-rails.spec
SRPM URL: http://tdawson.fedorapeople.org/rubygems/rubygem-compass-rails-1.0.3-2.fc18.src.rpm

License - Fixed
Doc subpackage - moved all suggested files in there
Requires: ruby - removed  (I'll have to start doing that from now on)

Test Suite:
I didn't notice that it had bypassed all the tests and returned with a positive outcome.  I wish it wouldn't do that.
I tried with minitest, building and installing all the dependancies (compass > 0.12, and rainbow), and after all that, it didn't run any tests either.

Even without the tests, this package cannot be installed, because it requires rubygem-compass > 0.12.2.  It looks like mmorsi is trying to get it working but it's currently failling on F19 (while it build on F18 for me)

http://koji.fedoraproject.org/koji/packageinfo?packageID=10224

(In reply to comment #4)
> Spec URL: http://tdawson.fedorapeople.org/rubygems/rubygem-compass-rails.spec
> SRPM URL:
> http://tdawson.fedorapeople.org/rubygems/rubygem-compass-rails-1.0.3-2.fc18.
> src.rpm
> 
> License - Fixed
> Doc subpackage - moved all suggested files in there
> Requires: ruby - removed  (I'll have to start doing that from now on)

Thank you. Will check that later.

> Test Suite:
> I didn't notice that it had bypassed all the tests and returned with a
> positive outcome.  I wish it wouldn't do that.
> I tried with minitest, building and installing all the dependancies (compass
> > 0.12, and rainbow), and after all that, it didn't run any tests either.
> 
> Even without the tests, this package cannot be installed, because it
> requires rubygem-compass > 0.12.2.  It looks like mmorsi is trying to get it
> working but it's currently failling on F19 (while it build on F18 for me)
> 
> http://koji.fedoraproject.org/koji/packageinfo?packageID=10224

Yes, I noticed that as well. Are you in contact with him? I'll continue with this review as soon as updated compass gets into Fedora.

I sent mmorsi an email today.  We'll see how quick it can be updated.
His package builds on F18, but it fails on the tests in rawhide (F19)

I'll make some noise on this when rubygem-compass is updated.

rubygem-compass 0.12.2 is now in Fedora 19, 18, and 17.
Please continue with the review.

(In reply to comment #4)
> Requires: ruby - removed  (I'll have to start doing that from now on)

I am negative about this change:
$ grep "Requires: *ruby$" rubygem-compass-rails.spec 
Requires:      ruby


> Test Suite:
> I didn't notice that it had bypassed all the tests and returned with a
> positive outcome.  I wish it wouldn't do that.
> I tried with minitest, building and installing all the dependancies (compass
> > 0.12, and rainbow), and after all that, it didn't run any tests either.

Hm, the test suite is insane :/ It looks more like test suite of Bundler then test suite of simple gem. ATM, I would suggest you to disable the test suite including all the BR and if you don't mind, please ask the upstream if it is possible to run the test suite with limited usage of bundler and just against Rails 3.2 and document the situation in .spec file.

BTW it would be better to run the test suite in .%{gem_instdir} if it will be possible one day in the future.

And I have on last minor nit:

* Keep the %{gem_instdir}/gemfiles
  - I would keep this files in -doc subpackage. Upstream distributes them, so
    let's keep them

Otherwise, the package looks good => APPROVED. Please fix the above mentioned issue prior importing package into Fedora.

New Package SCM Request
=======================
Package Name: rubygem-compass-rails0
Short Description: Integrate Compass into Rails 2.3 and up
Owners: tdawson
Branches: f17 f18
InitialCC:

WARNING: Requested package name rubygem-compass-rails0 doesn't match bug
summary rubygem-compass-rails

New Package SCM Request
=======================
Package Name: rubygem-compass-rails
Short Description: Integrate Compass into Rails 2.3 and up
Owners: tdawson
Branches: f17 f18
InitialCC:

Git done (by process-git-requests).

Vit, just a word on the version I'm building

- requires: ruby - removed (really removed this time)
- %{gem_instdir}/gemfiles - not removed, now in -doc
- Tests:
-- pushd ./%{gem_instdir}, with corresponding popd
-- commented out all tests with note to clean them up upstream

Thanks for the review.

rubygem-compass-rails-1.0.3-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/rubygem-compass-rails-1.0.3-3.fc18

rubygem-compass-rails-1.0.3-3.fc18 has been pushed to the Fedora 18 testing repository.

rubygem-compass-rails-1.0.3-3.fc18 has been pushed to the Fedora 18 stable repository.