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=453847 Hans de Goede <hdegoede@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hdegoede@xxxxxxxxxx --- Comment #18 from Hans de Goede <hdegoede@xxxxxxxxxx> 2009-02-16 04:46:11 EDT --- As discussed by mail, after I've reviewed 3 of your submission and deemed them approvable (the actual approving will happen when you get sponsored) I'll sponsor you. Full review done, looks good, I only have a few small nitpicks: MUST FIX -------- * You are missing: BuildRequires: perl, note that Requires will not get installed during the build, only BuildRequires (and visa versa during install) * Source code changes / conversions should be done in %prep, so please move these 3 lines: iconv -f iso-8859-1 -t utf-8 LICENSE > LICENSE.utf8 touch -c -r LICENSE LICENSE.utf8 mv LICENSE.utf8 LICENSE to %prep SHOULD FIX ---------- * Please add a comment as to why you are removing these files: rm $RPM_BUILD_ROOT%{_sbindir}/gpt-perl-version rm $RPM_BUILD_ROOT%{_datadir}/globus/globus_core-src.tar.gz rm $RPM_BUILD_ROOT%{_datadir}/globus/gpt_scripts_list * It is sort of standard to put %doc in %files the line below %defattr, instead of at the end * These 2 rpmlint messages: grid-packaging-tools.noarch: E: non-executable-script /usr/share/globus/aclocal/bootstrap.frg 0644 grid-packaging-tools.src: W: mixed-use-of-spaces-and-tabs (spaces: line 115, tab: line 1) Simple chmod 755 the bootstrap.frg file, and in your specfile either uses tabs everywhere to indent or spaces, do not mix the 2 note that the Fedora standard is sort of to use spaces. But you're free to use tabs if you prefer. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review