Bug 723125 - Review Request: jutils - Common utilities for the Java Gaming Interface
Summary: Review Request: jutils - Common utilities for the Java Gaming Interface
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jerry James
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 723427 723779
TreeView+ depends on / blocked
 
Reported: 2011-07-19 07:29 UTC by Guido Grazioli
Modified: 2011-07-26 13:55 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2011-07-26 13:43:20 UTC
Type: ---
Embargoed:
loganjerry: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Guido Grazioli 2011-07-19 07:29:52 UTC
Spec URL: 
http://guidograzioli.fedorapeople.org/packages/jutils/jutils.spec
SRPM URL: 
http://guidograzioli.fedorapeople.org/packages/jutils/jutils-1.0.1-1.20110719svn.fc16.src.rpm
Description: 
This is the utils project that contains useful shared functionality
for the other Java Games Initiative APIs.

Comment 1 Jerry James 2011-07-22 17:16:15 UTC
I'll take this review.

Comment 2 Jerry James 2011-07-22 17:49:52 UTC
I have two minor comments.  First, I think you should include the revision number in the subversion checkout comment, so that if upstream ever commits to the repository again, the comment will still lead to getting the same sources as used to build this package; i.e., make the comment read like this:

# svn export -r 30 https://svn.java.net/svn/jutils~svn/trunk jutils

Second, I question the usefulness of having /usr/share/javadoc/jutils contain a directory named "apidocs".  Wouldn't it be better to move the contents of apidocs up one level?  That is, I'm suggesting this way of installing the javadocs:

# javadoc
mkdir -p $RPM_BUILD_ROOT%{_javadocdir}/%{name}
cp -a target/site/apidocs  $RPM_BUILD_ROOT%{_javadocdir}/%{name}

Legend:
+: OK
-: must be fixed
=: should be fixed (at your discretion)
N: not applicable

MUST:
[+] rpmlint output:
$ rpmlint jutils.spec jutils jutils-javadoc
jutils.spec: W: invalid-url Source0: jutils-1.0.1.tar.xz
jutils.noarch: W: spelling-error %description -l en_US utils -> tills
jutils-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Avocados
2 packages and 1 specfiles checked; 0 errors, 3 warnings.

Those are all harmless.  Stupid spell checker. :-)
[+] follows package naming guidelines
[+] package meets the packaging guidelines
[+] package uses a Fedora approved license
[+] license field matches the actual license
[N] license file is included in %doc
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: I did my own checkout to compare.
[+] package builds on at least one primary arch (tried x86_64)
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[N] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[N] package contains a desktop file, uses desktop-file-install
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[=] query upstream for license text; good luck with that!  I don't really see the need to do this, since upstream provided a link to the license text.
[N] description and summary contains available translations
[+] package builds in mock: tried fedora-rawhide-i386
[+] package builds on all supported arches: tried i386 and x86_64
[+] package functions as described: minimal testing only
[+] sane scriptlets
[+] subpackages require the main package
[N] placement of pkgconfig files
[N] file dependencies versus package dependencies
[N] package contains man pages for binaries/scripts

Since this package meets all of the MUST items, it is approved.  I encourage you to consider the two points raised at the top before committing this package to git, though.

Comment 3 Jerry James 2011-07-22 17:59:04 UTC
(In reply to comment #2)
> Second, I question the usefulness of having /usr/share/javadoc/jutils contain a
> directory named "apidocs".  Wouldn't it be better to move the contents of
> apidocs up one level?  That is, I'm suggesting this way of installing the
> javadocs:
> 
> # javadoc
> mkdir -p $RPM_BUILD_ROOT%{_javadocdir}/%{name}
> cp -a target/site/apidocs  $RPM_BUILD_ROOT%{_javadocdir}/%{name}

Argh, no, I copied the wrong lines.  I meant *this* way of installing the javadocs:

# javadoc
mkdir -p $RPM_BUILD_ROOT%{_javadocdir}
cp -a target/site/apidocs  $RPM_BUILD_ROOT%{_javadocdir}/%{name}

Comment 4 Guido Grazioli 2011-07-24 17:11:22 UTC
Thanks for the review, I will apply both your proposals.

New Package SCM Request
=======================
Package Name: jutils
Short Description: Common utilities for the Java Gaming Interface
Owners: guidograzioli
Branches: f15
InitialCC:

Comment 5 Gwyn Ciesla 2011-07-24 19:23:42 UTC
Git done (by process-git-requests).

Comment 6 Guido Grazioli 2011-07-26 13:43:20 UTC
Built in rawhide


Note You need to log in before you can comment on or make changes to this bug.