Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 226438

Summary: Merge Review: struts
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Garrett Holmstrom <gholms>
Status: CLOSED DEFERRED QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: akurtako, dbhole, gholms
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-07-27 19:35:53 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Attachments:
Description Flags
Review for devel package struts-1.2.9-7.12.fc15 none

Description Nobody's working on this, feel free to take it 2007-01-31 21:02:30 UTC
Fedora Merge Review: struts

http://cvs.fedora.redhat.com/viewcvs/devel/struts/
Initial Owner: dbhole@redhat.com

Comment 1 Garrett Holmstrom 2010-11-18 06:24:30 UTC
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!

Comment 2 Garrett Holmstrom 2010-11-18 06:25:08 UTC
Created attachment 461224 [details]
Review for devel package struts-1.2.9-7.12.fc15

Comment 3 Alexander Kurtakov 2011-03-31 06:55:01 UTC
Garreth, 
The package is orphaned so if you care about it you should take it and start maintaining it.

Comment 4 Garrett Holmstrom 2011-03-31 07:11:13 UTC
I have no particular interest in this package.  If and when it is retired this merge review can be closed.

Comment 5 Alexander Kurtakov 2011-07-27 19:35:53 UTC
Struts has been deprecated so if it ever goes back in it will need a full review to happen thus closing this one.