Hide Forgot
Spec URL: http://mifo.sk/RPMS/rubygem-factory_girl.spec SRPM URL: http://mifo.sk/RPMS/rubygem-factory_girl-1.3.2-1.fc13.src.rpm Description: Framework and DSL for defining and using factories - less error-prone, more explicit, and all-around easier to work with than fixtures.
Will take this one. Overall looks good, some specific comments though * Can you change source0 to point to the official gem hosted at http://rubygems.org/downloads/%{name}-%{version}.gem * The LICENSE file should be part of the main package and marked as %doc. This is so that if only the main package is installed and not the docs subpackage (as usually is the case) the LICENSE is still included. http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text * Are sqlite3-ruby & activerecord really Requires or just BuildRequires (for tests)? Looking at the factory_girl source it seems like they are only pulled in for testing purposes. * Can you fix the Rakefile with a patch in the spec (eg %patch0) instead of the hotfix in %install section. * The %files section for both packages need default attributes: %defattr(-, root, root, -) Thanks for this submission.
(In reply to comment #1) Thanks! > Will take this one. Overall looks good, some specific comments though > > * Can you change source0 to point to the official gem hosted at > http://rubygems.org/downloads/%{name}-%{version}.gem FIXED. > > * The LICENSE file should be part of the main package and marked as %doc. This > is so that if only the main package is installed and not the docs subpackage > (as usually is the case) the LICENSE is still included. > > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text FIXED. > > * Are sqlite3-ruby & activerecord really Requires or just BuildRequires (for > tests)? Looking at the factory_girl source it seems like they are only pulled > in for testing purposes. FIXED (Runtime dependencies removed) > * Can you fix the Rakefile with a patch in the spec (eg %patch0) instead of the > hotfix in %install section. FIXED. > * The %files section for both packages need default attributes: > %defattr(-, root, root, -) FIXED. =================== 1.3.2-1 ==================== http://koji.fedoraproject.org/koji/taskinfo?taskID=2532551 * Wed Oct 13 2010 Michal Fojtik <mfojtik@redhat.com> - 1.3.2-2 - Rakefile fixing moved to a separate patch - Fixed unneeded Requires - Fixed directory ownership on doc subpackage - README and LICENSE moved back to main package Spec URL: http://mifo.sk/RPMS/rubygem-factory_girl.spec SRPM URL: http://mifo.sk/RPMS/rubygem-factory_girl-1.3.2-2.fc13.src.rpm
Everything looks good save the patch for the Rakefile, the proper way to do it would be something like this in the %prep section: %setup -q -c -T mkdir -p ./%{gemdir} gem install --local --install-dir ./%{gemdir} \ --force --rdoc %{SOURCE0} pushd ./%{geminstdir} %patch0 popd and then in the %install section %install rm -rf %{buildroot} mkdir -p %{buildroot}%{gemdir} cp -a .%{gemdir}/* %{buildroot}%{gemdir} See rubygem-activerecord as an example of how to do this http://pkgs.fedoraproject.org/gitweb/?p=rubygem-activerecord.git;a=blob;f=rubygem-activerecord.spec Other than that, everything else looks great. rpmlint is fine, it builds on koji, and I did a surface functionality test.
(In reply to comment #3) > Everything looks good save the patch for the Rakefile, the proper way to do it > would be something like this in the %prep section: > > %setup -q -c -T > > mkdir -p ./%{gemdir} > gem install --local --install-dir ./%{gemdir} \ > --force --rdoc %{SOURCE0} > > pushd ./%{geminstdir} > %patch0 > popd > > > > and then in the %install section > > %install > rm -rf %{buildroot} > mkdir -p %{buildroot}%{gemdir} > cp -a .%{gemdir}/* %{buildroot}%{gemdir} > > > See rubygem-activerecord as an example of how to do this Thanks for noticing that, it looks definitely better right now. Fixed. > http://pkgs.fedoraproject.org/gitweb/?p=rubygem-activerecord.git;a=blob;f=rubygem-activerecord.spec > > > Other than that, everything else looks great. rpmlint is fine, it builds on > koji, and I did a surface functionality test. ==================== 1.3.2-2 ==================== * Thu Oct 14 2010 Michal Fojtik <mfojtik@redhat.com> - 1.3.2-3 - Replaced path with path macro Spec URL: http://mifo.sk/RPMS/rubygem-factory_girl.spec SRPM URL: http://mifo.sk/RPMS/rubygem-factory_girl-1.3.2-2.fc13.src.rpm
It seems you forgot to bump the 'release' in the last spec to '3'. That;s the last nit that needs to be fixed though, everything else looks good. APPROVED rubygem-factory_girl [mmorsi]
Thanks! New Package SCM Request ======================= Package Name: rubygem-factory_girl Short Description: Framework and DSL for defining and using model instance factories Owners: mfojtik Branches: f12 f13 f14
Git done (by process-git-requests).
rubygem-factory_girl-1.3.2-3.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/rubygem-factory_girl-1.3.2-3.fc13
rubygem-factory_girl-1.3.2-3.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update rubygem-factory_girl'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/rubygem-factory_girl-1.3.2-3.fc13
rubygem-factory_girl-1.3.2-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.