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 <mefos...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |mefos...@gmail.com
         AssignedTo|nob...@fedoraproject.org    |mefos...@gmail.com




--- Comment #2 from Mary Ellen Foster <mefos...@gmail.com>  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@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review

Reply via email to