[Bug 226438] New: Merge Review: struts

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226438

Summary: Merge Review: struts
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail]

Fedora Merge Review: struts

http://cvs.fedora.redhat.com/viewcvs/devel/struts/
Initial Owner: [e-mail]

[Bug 226438] Merge Review: struts

[Red Hat Bugzilla]

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  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #1 from Garrett Holmstrom  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!

[Bug 226438] Merge Review: struts

[Red Hat Bugzilla]

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

--- Comment #2 from Garrett Holmstrom  2010-11-18 01:25:08 EST ---
Created attachment 461224
  --> https://bugzilla.redhat.com/attachment.cgi?id=461224
Review for devel package struts-1.2.9-7.12.fc15

[Bug 226438] Merge Review: struts

[Red Hat Bugzilla]

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

Alexander Kurtakov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

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

[Bug 226438] Merge Review: struts

[Red Hat Bugzilla]

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

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

[Bug 226438] Merge Review: struts

[Red Hat Bugzilla]

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

Alexander Kurtakov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |DEFERRED
               Flag|fedora-review?              |
        Last Closed|                            |2011-07-27 15:35:53

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