[Bug 589504] Review Request: apache-commons-compress - Java API for working with tar, zip and bzip2 files

[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=589504

--- Comment #12 from Sandro Mathys <sm at sandro-mathys.ch> 2010-05-10 06:37:14 EDT ---
Spec URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress.spec
SRPM URL:
http://red.fedorapeople.org/SRPMS/apache-commons-compress-1.0-4.fc12.src.rpm

Thanks for the review Alexander!

(In reply to comment #11)
> Other problems:
> * You should not install in %build section. Please just remove the "install -d
> $MAVEN_REPO_LOCAL" line.

This line looks important to me. Also, I see no reason not to install something
into the unpacked sources if necessary for the build. After all, mvn-jpp will
write to that dir as well. Also, the maven template from the guidelines does
the same (with mkdir instead of install):
http://fedoraproject.org/wiki/Packaging:Java#maven_2

If you have a reason to believe mkdir would fit better I'm willing to replace
the install by that, tho.

> * All your symlinks are broken "ln -s %{name}-%{version}.jar" should be "ln -s
> %{shortname}-%{version}.jar

Awww, very good catch! Should have paid more attention as I changed that
symlinking part a bit recently :/ FIXED.

> * You should not use %define but %global. See
> http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Right, forgot to check that for the new renamed version - and the jakarta-*
thingy is older than this rule :) FIXED.

> * You can make use of the %{name} macro on a few places (if you want to) -
> Javadoc subpackage Summary and Description.    

True, FIXED.

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