[Bug 464781] Review Request: flexdock - Java docking UI element. First package.

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


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





--- Comment #30 from Dominik 'Rathann' Mierzejewski <rpm at greysector.net>  2009-02-15 19:19:09 EDT ---
Created an attachment (id=331996)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=331996)
Suggested specfile fixes

Full review:

rpmlint is clean
$ rpmlint .
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

    * MUST: The package must meet the Packaging Guidelines.

%define _jdkdir %{_jvmdir}/java-1.6.0-openjdk-1.6.0.0

I suggest using simply

%{_jvmdir}/java-1.6.0

instead, which will allow you to drop the

JDK_DIR=`echo %{_jdkdir} | sed 's!/$!!'`.`uname -m`

hack later (see attached patch). Is java-1.6 (not older and not newer) strictly
required?

Patch0: flexdock-jni-patch.patch

+        File file = new File("%{_libdir}/%{name}/");^

That '/' at the end is not necessary. Also the patch file name has a redundant
'patch' in it, same for
others.


BuildRequires: jpackage-utils

is listed twice.


#Override the build file's default hard-coded paths
if [ x"$JAVACMD" != x"" ] ; then
        echo "using RPM jnidir"  >> tmpLog
        echo sdk.home=%{_jnidir}-`$JAVACMD -version` > workingcopy.properties
else
        if [ x"$JAVA_HOME" != x"" ] ; then
                echo "Using JAVA_HOME env. var. :" $JAVA_HOME >> tmpLog
                echo "sdk.home=$JAVA_HOME" > workingcopy.properties
        else
                JDK_DIR=%{_jdkdir}
                echo "Relying on spec file buildpath: $JDK_DIR " >> tmpLog
                echo "sdk.home=$JDK_DIR"  > workingcopy.properties
        fi
fi

Why is the above necessary instead of:
echo "sdk.home=%{_jvmdir}/java-1.6.0" > workingcopy.properties

You could lose the %define _jdkdir at the beginning of the specfile then, too.


Also see the attached patch for more cosmetic fixes.

    * MUST: The package must be licensed with a Fedora approved license and
meet the Licensing Guidelines.

Most files have no licencing information while others are licenced under MIT
licence. Given the presence
of LICENSE.txt, this is OK, but please ask upstream to include licencing
information at the top of each
source file

    * MUST: The License field in the package spec file must match the actual
license.

#Licence is MIT on their website, Apache in their LICENSE.txt 
License: MIT and ASL 2.0

Wrong. LICENSE.txt is actually word-for-word MIT:
https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense

    * MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL. Reviewers should use md5sum for this task.
If no upstream URL can be specified for this package, please see the Source URL
Guidelines for how to deal with this.

88fd43d7d8db92e9480200c316e55056  flexdock-0.5.1-src.zip

    * SHOULD: The reviewer should test that the package builds in mock.

    * SHOULD: The package should compile and build into binary rpms on all
supported architectures.

Doesn't build on F-9/x86_64 and F-9/i386 (java bug?).

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