Hide Forgot
SPEC: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails.spec SRPM: http://bkabrda.fedorapeople.org/haml-rails/rubygem-haml-rails-0.3.4-1.fc15.src.rpm Description: Haml-rails provides Haml generators for Rails 3. It also enables Haml as the templating engine for you, so you don't have to screw around in your own application.rb when your Gemfile already clearly indicated what templating engine you have installed. Hurrah. Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3344497
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.