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=462521 Mary Ellen Foster <mefoster@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mefoster@xxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |mefoster@xxxxxxxxx --- Comment #2 from Mary Ellen Foster <mefoster@xxxxxxxxx> 2009-02-06 10:28:00 EDT --- (Using Jason Tibbitts' review template from http://fedoraproject.org/wiki/User:Tibbs/Review_Template) [-] source files match upstream: I followed the instructions in the spec file and ended up with something with a different md5sum than the file in the SRPM. The readme.txt and gpl.txt files do match though You could just use the upstream .zip file directly, though. Just change your %setup line to %setup -q -c %{name}-%{version} and it'll create the directory when it needs to. [+] package meets naming and versioning guidelines. [+] specfile is properly named, is cleanly written and uses macros consistently. [+] dist tag is present. [+] build root is correct. [+] license field matches the actual license. [+] license is open source-compatible. [+] license text included in package. [-] latest version is being packaged. It looks like upstream has released 0.12.5 [+] BuildRequires are proper. [+] compiler flags are appropriate. [+] %clean is present. [-] package builds in mock. error: %patch without corresponding "Patch:" tag You should have "%patch0", not "%patch", on line 62 For the remainder of this review I made this change [+] package installs properly. [+] debuginfo package looks complete. [-] rpmlint is silent. One warning to deal with: simplyhtml.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 20) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. Other warnings are about Group tags or confusion with the gcj noarch-ness [+] final provides and requires are sane: Provides: simplyhtml-0.12.3.jar.so simplyhtml = 0:0.12.3-2.fc10 simplyhtml(x86-32) = 0:0.12.3-2.fc10 Requires: /bin/sh gnu-regexp java java-gcj-compat >= 1.0.31 javahelp2 jpackage-utils libc.so.6 libc.so.6(GLIBC_2.1.3) libdl.so.2 libgcc_s.so.1 libgcc_s.so.1(GCC_3.0) libgcj_bc.so.1 libm.so.6 libpthread.so.0 librt.so.1 libz.so.1 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) [+] no shared libraries are added to the regular linker search paths. [+] owns the directories it creates. [+] doesn't own any directories it shouldn't. [+] no duplicates in %files. [+] file permissions are appropriate. [+] gcj scriplets are correct [+] code, not content. [+] documentation is small, so no -docs subpackage is necessary. [+] %docs are not necessary for the proper functioning of the package. [+] no headers. [+] no pkgconfig files. [+] no libtool .la droppings. [-] Package doesn't run I recommend creating a small shell script to run the program with the correct CLASSPATH -- if you just try to run the jar file, it doesn't find the gnu-regexp classes. Something like this: #!/bin/sh exec java -cp `build-classpath gnu-regexp javahelp2 simplyhtml` \ com.lightdev.app.shtm.App [-] Other source files are included: Please check the status and the necessity of using the files in src/com/sun and de/calcom as they appear to come from other projects. -- 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