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 Alexander Kurtakov <akurtako@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+ --- Comment #13 from Alexander Kurtakov <akurtako@xxxxxxxxxx> 2010-05-10 07:26:35 EDT --- (In reply to comment #12) > 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. Well, using install in %build section can make someone believe that you are putting smth in the buildroot, which you are certainly not doing. Using mkdir is preferred to not confuse people. > > > * 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. Thanks, This package is APPROVED. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review