[Bug 453847] Review Request: grid-packaging-tools - The Grid Packaging Tools (GPT)

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


Hans de Goede <hdegoede at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hdegoede at redhat.com




--- Comment #18 from Hans de Goede <hdegoede at redhat.com>  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.