Bug 737511

I'll take this one.

* Test suite execution
  - I would suggest to replace the %check section with following content which
    should provide similar results:

    pushd .%{geminstdir}
    testrb -Itest test/lib/generators/haml/*_test.rb
    popd

* Provides differs from upstream
  - Is there any reason why you require rubygem(rails) instead of the gem set
    which is used by upstream? Requires should be kept minimal as possible. 
    The test suite passes if rubygem(rails) dependency is replaced by:

    BuildRequires: rubygem(actionmailer)
    BuildRequires: rubygem(activerecord)

    Are these gems really hard requirement? It seems that they are optional.
    Please investigate.

* Keep the test files
  - If the test suite is part of the gem, please keep the tests in -doc
    subpackage.
  - The Gemfile should be also kept in -doc subpackage, as it is intended to be
    used by the test suite

(In reply to comment #2)
> * Test suite execution
>   - I would suggest to replace the %check section with following content which
>     should provide similar results:
> 
>     pushd .%{geminstdir}
>     testrb -Itest test/lib/generators/haml/*_test.rb
>     popd
> 

- Done

> * Provides differs from upstream
>   - Is there any reason why you require rubygem(rails) instead of the gem set
>     which is used by upstream? Requires should be kept minimal as possible. 
>     The test suite passes if rubygem(rails) dependency is replaced by:
> 
>     BuildRequires: rubygem(actionmailer)
>     BuildRequires: rubygem(activerecord)
> 
>     Are these gems really hard requirement? It seems that they are optional.
>     Please investigate.
>

- Yes, the tests pass without rubygem(rails) and with these two included, but you also need to include BR: rubygem(railties)
 
> * Keep the test files
>   - If the test suite is part of the gem, please keep the tests in -doc
>     subpackage.
>   - The Gemfile should be also kept in -doc subpackage, as it is intended to be
>     used by the test suite

- Done

SPEC: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails.spec
SRPM: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails-0.3.4-2.fc15.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3347553

* %doc macro in -doc sub-package
  - I would suggest to mark by %doc macro only documents. It means that the test
    suite and the Gemfile should not be marked as doc.
  - There are also other opinions, such as that everything in -doc sub-package
    is documentation and there is no need to mark anything as a documentation.
    However I do not share this POV.

* Substantially different R and BR
  - Requires and BuildRequires differ substantially, so I wonder what is the
    reason for the difference. I guess that ActiveRecord and ActionMailer
    are just optional dependencies, but I'd like to be sure.

(In reply to comment #4)
> * %doc macro in -doc sub-package
>   - I would suggest to mark by %doc macro only documents. It means that the
> test
>     suite and the Gemfile should not be marked as doc.
>   - There are also other opinions, such as that everything in -doc sub-package
>     is documentation and there is no need to mark anything as a documentation.
>     However I do not share this POV.
>
I definitelly agree, that marking tests and Gemfile as doc wasn't appropriate - fixed.

> * Substantially different R and BR
>   - Requires and BuildRequires differ substantially, so I wonder what is the
>     reason for the difference. I guess that ActiveRecord and ActionMailer
>     are just optional dependencies, but I'd like to be sure.

ActiveRecord and ActionMailer are really required during tests, but I managed to remove some Requires (concretely activesupport and actionpack), which are already required by railties.

SPEC: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails.spec
SRPM: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails-0.3.4-3.fc15.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3347929

Additional info:
- controller generator can be used to generate a controller without actionmailer and activerecord
- scaffold generator can be used with other ORM (tested with datamapper)
- mailer generator always generates a subclass of ActionMailer::Base, so it seems that for its proper usage, ActionMailer is required

Summary: I would recommend adding Requires: rubygem(actionmailer) even though haml-rails can work with controllers and scaffolds even without it.

* I believe that not every Rails application has to use ActionMailer for sending
  emails, so the actionmailer is just optional from haml-rails POV.

* Remove unused "testdir" macro

* %{geminstdir}/haml-rails.gemspec should not be marked as %doc

Otherwise the package looks good. So please fix this nits and go ahead with the Git request. This package is APPROVED.

New Package SCM Request
=======================
Package Name: rubygem-haml-rails
Short Description: Haml generators for Rails 3
Owners: bkabrda
Branches: 
InitialCC:

Git done (by process-git-requests).

Already available in Rawhide.