[Bug 516466] Review Request: sys_basher - multi-threaded hardware tester

bugzilla at redhat.com bugzilla at redhat.com
Mon Aug 10 18:00:55 UTC 2009


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=516466





--- Comment #13 from Jussi Lehtola <jussi.lehtola at iki.fi>  2009-08-10 14:00:54 EDT ---
(In reply to comment #12)
> I've changed the license from the original BSD to the new BSD. I've also fixed
> the Makefile and the spec file. My website has been updated to reflect these
> changes. Please look it over.  

As I advised in comment #6, please don't attach the specs or tarballs, just
paste the relevant links to the spec & srpm. In this case:

http://www.polybus.com/sys_basher/sys_basher.spec
http://www.polybus.com/sys_basher/sys_basher-1.1.17-3.fc11.src.rpm

would have done nicely.

**

Now the license tag is again incorrect, it should be just "BSD" instead of "BSD
no advertising". Please see 
http://fedoraproject.org/wiki/Licensing#Software_License_List

**

You are still missing BuildRequires, making the package not build under mock.

+ make -j4
gcc -c -O2 -Wuninitialized -Wformat -Wno-multichar -Wreturn-type -Wswitch -g 
sys_main.c
gcc -c -O2 -Wuninitialized -Wformat -Wno-multichar -Wreturn-type -Wswitch -g 
sys_utils.c
gcc -c -O2 -Wuninitialized -Wformat -Wno-multichar -Wreturn-type -Wswitch -g 
sys_disk.c
gcc -c -O2 -Wuninitialized -Wformat -Wno-multichar -Wreturn-type -Wswitch -g 
sys_kernel.c
In file included from sys_disk.c:28:
In file included from sys_utils.c:28:
sys_basher.h:37:21: error: ncurses.h: No such file or directory
sys_basher.h:37:21: error: ncurses.h: No such file or directory
In file included from sys_main.c:28:
sys_basher.h:37:21: error: ncurses.h: No such file or directory

You might want to add a clause for ncurses.h, too. Or maybe switch to using
e.g. GNU autotools to configure the build process.

**

As one can see, the build process does not use the Fedora optimization flags.
Use
 make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS"
to fix this.

**

On a side note, I'm wondering why you have many variables for the same thing in
the Makefile. You should have only the standard variables CC and CFLAGS, and
call them in the make process. I'll attach a patch in a moment.

**

You are missing changelog entries altogether. Add entries for every change and
release you've done so far.

**

All of the tarball files (except the spec file) are still executable. Please
remove the executable flags from the files in the upstream tarball.

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




More information about the Fedora-package-review mailing list