Bug 588654 - Review Request: plexus-component-api - Plexus Component API
Review Request: plexus-component-api - Plexus Component API
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Overholt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-04 04:34 EDT by Yong Yang
Modified: 2010-05-19 15:31 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-05-19 15:31:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
overholt: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)
suggested fixes (1.06 KB, patch)
2010-05-05 10:18 EDT, Andrew Overholt
no flags Details | Diff

  None (edit)
Comment 1 Andrew Overholt 2010-05-04 14:38:06 EDT
Thanks for the submission.  My review is below.

* = okay, X = work to be done

* naming fine
X release incorrect.  see:

  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release

X drop the explicit Epoch (it's unnecessary)
X license tag incorrect (it should be ASL 2.0):  see

  http://fedoraproject.org/wiki/Licensing#SoftwareLicenses 

X source appears to have been generated from a different tag.  The difference
  is inconsequential but I'd prefer if it was re-generated.  See the diff here:

  http://overholt.fedorapeople.org/plexus-component-api-1.0-alpha-15.diff

X Requires need to be fixed:  add Requires(post) and Requires(postun) on
  jpackage-utils

* macros okay (but why mix %{__rm} and plain rm?)

X should there be an un-versioned copy of the JAR?

* rpmlint warnings are okay (license covered above and others are ignorable)

$ rpmlint ../SRPMS/plexus-component-api-1.0-alpha15.1.src.rpm 
plexus-component-api.src: W: invalid-license Apache Software License 2.0
plexus-component-api.src: W: invalid-url Source0:
plexus-component-api-1.0-alpha-15.tar.gz
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint ../RPMS/noarch/plexus-component-api-1.0-alpha15.1.fc12.noarch.rpm 
plexus-component-api.noarch: W: invalid-license Apache Software License 2.0
plexus-component-api.noarch: W: no-documentation
plexus-component-api.noarch: W: non-conffile-in-etc /etc/maven/fragments/plexus-component-api
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint ../RPMS/noarch/plexus-component-api-javadoc-1.0-alpha15.1.fc12.noarch.rpm 
plexus-component-api-javadoc.noarch: W: invalid-license Apache Software License 2.0
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 2 Yong Yang 2010-05-05 03:39:08 EDT
Fixed all issues except "source appears to have been generated from a different tag" which I think don't need to fix,it was just caused by the auto-generated svn placeholder $Data$ for different timezone.

Please review again.

Spec URL: http://yyang.fedorapeople.org/plexus/plexus-component-api.spec
SRPM URL: http://yyang.fedorapeople.org/plexus/plexus-component-api-1.0-0.1.alpha15.src.rpm


FYI the different line in http://overholt.fedorapeople.org/plexus-component-api-1.0-alpha-15.diff:
- * @version CVS $Revision: 4796 $ $Date: 2006-11-24 05:24:11 +0800 (Fri, 24 Nov 2006) $
+ * @version CVS $Revision: 4796 $ $Date: 2006-11-23 16:24:11 -0500 (Thu, 23 Nov 2006) $
Comment 3 Andrew Overholt 2010-05-05 10:18:29 EDT
Created attachment 411629 [details]
suggested fixes

It looks good, thanks for the fixes.  Please drop the blank line immediately following %changelog.

There are two remaining rpmlint issues:

plexus-component-api.noarch: W: incoherent-version-in-changelog 0:1.0-0.1.alpha15 ['1.0-0.1.alpha15.fc12', '1.0-0.1.alpha15']
plexus-component-api.noarch: W: no-documentation
plexus-component-api.noarch: W: non-conffile-in-etc /etc/maven/fragments/plexus-component-api
plexus-component-api.noarch: W: dangling-relative-symlink /usr/share/java/plexus-component-api-1.0-alpha-15.jar *-%{prject_version}*

You need something like the attached diff.

Also, I see that we have /usr/share/java/plexus.  Please put the JARs in there.
Comment 4 Yong Yang 2010-05-06 09:14:54 EDT
Fixed: 1.incoherent-version-in-changelog;2.drop the blank line immediately
following %changelog;3.typo %{prject_version}; 4. jars to plexus directory


Please review again.

Spec URL: http://yyang.fedorapeople.org/plexus/plexus-component-api.spec
SRPM URL:
http://yyang.fedorapeople.org/plexus/plexus-component-api-1.0-0.1.alpha15.src.rpm
Comment 5 Andrew Overholt 2010-05-06 09:51:34 EDT
I still get incoherent-version-in-changelog.  Are we running the same rpmlint?

$ rpmlint -V
rpmlint version 0.95 Copyright (C) 1999-2007 Frederic Lepied, Mandriva

Other than the incorrect version in %changelog, things are fine.  Correct that and this will be approved.  Thanks.
Comment 6 Yong Yang 2010-05-07 04:40:19 EDT
I have upgraded rpmlint to 0.91 which is the latest version I can find for RHEL5.

FYI:
---
[yyang@localhost SPECS]$ rpmlint -V
rpmlint version 0.91 Copyright (C) 1999-2007 Frederic Lepied, Mandriva
[yyang@localhost SPECS]$ rpmlint plexus-component-api.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
---

Please review again.
Spec URL: http://yyang.fedorapeople.org/plexus/plexus-component-api.spec
SRPM URL:
http://yyang.fedorapeople.org/plexus/plexus-component-api-1.0-0.1.alpha15.src.rpm

If you still get "incorrect version in %changelog", could you please help look into the SPEC file, and tell me how to fix it, many thanks.
Comment 7 Peter Lemenkov 2010-05-07 05:54:18 EDT
Please, also add necessary requires for owner of the used directories - jpackage-utils (owner of %{_mavenpomdir}, %{_mavendepmapfragdir} and %{_javadocdir}).

Unfortunately, due to overall poor quality of java-related packages. I can't find who is an owner of %{_javadocdir}/plexus/ and %{_javadir}/plexus. I've got numerous matches, but don't have enough knowledge to pick correct one.
Comment 8 Andrew Overholt 2010-05-07 10:25:55 EDT
Thanks, Yong, that rpmlint error is gone now.

Peter, thanks for bringing up the issue with directory ownership.  Everyone appreciates your help in getting the packages into a better state so please continue helping in this area :)

Yong, please do as Peter asks and add jpackage-utils as a Requires.  While the chain of dependencies would take care of this, it's just as valid to have the direct Requires.  Thanks.
Comment 9 Yong Yang 2010-05-09 23:23:11 EDT
Added 3 Requires
Requires:          java >= 1:1.6.0
Requires:          plexus-utils
Requires:          jpackage-utils

Please review again.
Spec URL: http://yyang.fedorapeople.org/plexus/plexus-component-api.spec
SRPM URL:
http://yyang.fedorapeople.org/plexus/plexus-component-api-1.0-0.1.alpha15.src.rpm

for directories %{_javadocdir}/plexus/ and %{_javadir}/plexus, I guess plexus-utils is the correct owner.
Comment 10 Andrew Overholt 2010-05-17 08:55:29 EDT
Thanks, Yong.  Everything is now okay so this package is APPROVED.  Please follow the next steps here:

https://fedoraproject.org/wiki/Package_Review_Process#Review_Process
Comment 11 Yong Yang 2010-05-17 09:26:43 EDT
New Package CVS Request
=======================
Package Name: plexus-component-api
Short Description: Plexus Component API
Owners: yyang
Branches: devel
InitialCC: overholt huwang liweinan
Comment 12 Dennis Gilmore 2010-05-18 14:33:34 EDT
CVS Done
Comment 13 Alexander Kurtakov 2010-05-19 15:31:00 EDT
Koji build:
http://koji.fedoraproject.org/koji/packageinfo?packageID=10364

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