[Bug 640627] Review Request: rubygem-factory_girl - Framework and DSL for defining and using model instance factories

[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=640627

--- Comment #2 from Michal Fojtik <mfojtik at redhat.com> 2010-10-13 11:01:09 EDT ---
(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 at 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

-- 
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.