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 Mohammed Morsi <mmorsi@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mmorsi@xxxxxxxxxx --- Comment #1 from Mohammed Morsi <mmorsi@xxxxxxxxxx> 2010-10-12 22:20:05 EDT --- 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. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review