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=721144 Mark McLoughlin <markmc@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |markmc@xxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |markmc@xxxxxxxxxx Flag| |fedora-review? --- Comment #1 from Mark McLoughlin <markmc@xxxxxxxxxx> 2011-07-18 04:47:59 EDT --- Looks good overall. I've only a bunch of fairly minor comments rpmlint shows no problems $> rpmlint imagefactory.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $> rpmlint imagefactory-0.3.1-1.fc14.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. other comments: - use %{version} instead of 0.3.1 in the source URL - license is 'ASL 2.0' not GPLv2 - looking at rpm's GROUPS file, I think I'd go for Applications/System instead of Development/Libraries - URL instead of Url - I don't think euca2ools is actually required; we run all the euca- commands inside EC2 instances not locally - it also doesn't look like python-suds is a direct dependency; psphere needs it alright, but not imagefactory directly - no need for python BR, python-setuptools is enough - use %{__python} instead of python - like in psphere, just do: %install %{__python} setup.py install --root=$RPM_BUILD_ROOT --skip-build - need to require chkconfig for post and preun and initscripts for preun http://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets - looks like you also forgot %{name} in the service stop line i.e. - /sbin/service stop >/dev/null 2>&1 - /sbin/service %{name} stop >/dev/null 2>&1 - I don't think condrestart in %postun is appropriate in our case; e.g. we don't want to cancel running builds or pushes when doing an update. For example, koji doesn't do this - Use %{_initddir} not %{_initrddir} http://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem -- 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