Bug 226438 - Merge Review: struts
Summary: Merge Review: struts
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Garrett Holmstrom
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 21:02 UTC by Nobody's working on this, feel free to take it
Modified: 2011-07-27 19:35 UTC (History)
3 users (show)

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 (Terms of Use)
Review for devel package struts-1.2.9-7.12.fc15 (32.72 KB, text/plain)
2010-11-18 06:25 UTC, Garrett Holmstrom
no flags Details

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.


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