[Bug 226438] Merge Review: struts
bugzilla at redhat.com
bugzilla at redhat.com
Thu Nov 18 06:24:32 UTC 2010
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=226438
Garrett Holmstrom <gholms at fedoraproject.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |gholms at fedoraproject.org
AssignedTo|nobody at fedoraproject.org |gholms at fedoraproject.org
Flag| |fedora-review?
--- Comment #1 from Garrett Holmstrom <gholms at fedoraproject.org> 2010-11-18 01:24:30 EST ---
It looks like this package hasn't seen any updates even though the 1.3 series
went stable well over a year ago; is it used any more?
The most important issues with this package follow. I will also attach a full
review.
- rpmlint output not covered in the rest of the review
Documentation packages should have group "Documentation", not
"Development/Documentation"
Why do you clean out the buildroot in %prep?
- License files installed when any subpackage combination is installed
The javadoc packages don't pull in anything that contains LICENSE.txt. I know,
it's silly. Sorry.
- Sources match upstream unless altered to fix permissibility issues
Though the tarball has been altered for redistribution, the spec file need to
contain instructions for building such a tarball from upstream's.
- File permissions are sane
/var/lib/tomcat5/webapps/struts-documentation/download.cgi 0644
Should this one be 0755?
- Scriptlets are sane
The old javadoc %post/%ghost procedure is now prohibited by both JPackage the
Java guidelines.
- Config files marked with %config
/etc/tomcat5/Catalina/localhost/*
These should be %config(noreplace), right?
- %global instead of %define where appropriate
Everywhere %define appears at the top should have %global instead.
- Patches link to upstream bugs/comments/lists or are otherwise justified
Please add some commentary about what the patches fix.
- javadoc subpackage is noarch on > EL5
Since you aren't building for EPEL you should make all the javadoc subpackages
noarch.
- BuildRequires java-devel, jpackage-utils
BuildRequires: java-devel missing
- Requires java >= $version, jpackage-utils
BuildRequires: java missing
BuildRequires: jpackage-utils missing
These are part of the minimum required by the Java guidelines.
- No class-path elements in JAR manifests
/usr/share/java/struts-1.2.9.jar
The Java guidelines recommend using sed to remove classpath elements prior to
JAR creation.
I hope that helps!
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
More information about the package-review
mailing list