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@xxxxxxxxxxxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review