[Bug 310641] Review Request: GREYCstoration - An image denoising and interpolation tool

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

Summary: Review Request: GREYCstoration - An image denoising and interpolation tool


https://bugzilla.redhat.com/show_bug.cgi?id=310641





------- Additional Comments From packages at amiga-hardware.com  2007-10-01 18:26 EST -------
1. It's usual to begin Patches at 0 and not 1. Not a problem but for the sake of
commonality you might want to change it.


2. Patches should ideally include a brief descriptor in the filename, for
example changing
GREYCstoration-2.5.2.1.patch to GREYCstoration-2.5.2.1-dontstrip.patch


3. "non standard" buildroot. You should probably change this to
 %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)


4. Extraneous options in the %build section. Is there a reason why you're
defining  -DSUPPORT_LH7 -DMKSTEMP?


5. Your package will almost certainly fail to build on ppc64 and x86_64 because
the makefile hard codes lib when they use lib64. One solution might be to use
%ifarch statements and sed to substitute lib for lib64 where appropriate on
64bit archs.


6. Unnecessary options for %setup, just use:

%setup -q


7. Unncessary buildrequire: gimp


8. Consider also installing GREYCstoration_gui.tcl with appropriate icon and in
a sub package (eg GREYCstoration-gui) so it's not forced on anyone who just
wants the command line or gimp version.


9. The makefile is broken with regards to parallel building. It doesn't cause
the buildjob to fail but reverts to -j1 anyway. Probably because make doesn't
realise it's calling itself. Ideally this should be fixed, but if not then drop
%{?_smp_mflags} and add a comment in the SPEC about it.



-- 
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, or are watching someone who is.