Bug 251361 - Review Request: compat-expat1 - library compat package for expat 1.x
Review Request: compat-expat1 - library compat package for expat 1.x
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 195888
  Show dependency treegraph
 
Reported: 2007-08-08 11:38 EDT by Joe Orton
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-22 16:49:32 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Joe Orton 2007-08-08 11:38:51 EDT
Spec URL: http://jorton.fedorapeople.org/expat/compat-expat1.spec
SRPM URL: http://jorton.fedorapeople.org/expat/compat-expat1-1.95.8-1.src.rpm
Description: library compat package for expat 1.x

This is a library compat package for expat 1.x, to go along with the forthcoming  expat soname bump with the upgrade to 2.x.  (expat is such a popular library, it is certainly worth having a compat package)
Comment 1 Joe Orton 2007-08-08 16:41:21 EDT
Updated versions:

Spec URL: http://jorton.fedorapeople.org/expat/compat-expat1.spec
SRPM URL: http://jorton.fedorapeople.org/expat/compat-expat1-1.95.8-2.src.rpm

have fixed the License tag.
Comment 2 Joe Orton 2007-08-21 10:59:42 EDT
This is essentially a renaming of the expat package with a stripped %files.  Not
sure it needs to go through a separate spec review.
Comment 3 Joe Orton 2007-08-21 11:00:59 EDT
New Package CVS Request
=======================
Package Name: compat-expat1
Short Description: Library compatibility package for expat 1
Owners: jorton
Branches: devel
InitialCC: 
Commits by cvsextras: no
Comment 4 Kevin Fenzi 2007-08-21 19:34:53 EDT
Yeah, everything should get reviewed. In this case it would be good to do to
make sure it works fine installed along side the non-compat version. 

I can probibly review it in a while if no one beats me to it...
Unsetting flags for now. 
Comment 5 Kevin Fenzi 2007-08-22 01:03:35 EDT
ok, look for a full review in a bit here.
Comment 6 Kevin Fenzi 2007-08-22 01:14:20 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (MIT)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
aff487543845a82fe262e6e2922b4c8e  expat-1.95.8.tar.gz
aff487543845a82fe262e6e2922b4c8e  expat-1.95.8.tar.gz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Spec has needed ldconfig in post and postun
OK - .la files are removed.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
See below - Should have dist tag

Issues:

1. Does this package need %makeinstall?
You shouldn't use it if you can avoid it. See:
http://fedoraproject.org/wiki/PackagingDrafts/MakeInstall

2. The URL in the Source line looks wrong...
http://download.sourceforge.net/expat/expat-%{version}.tar.gz
should be
http://downloads.sourceforge.net/expat/expat-%{version}.tar.gz
(note the plural).

3. By no means a blocker, but there's no dist tag.
Since this is a compat package and hopefully won't be around
for a long time it's probibly fine not to bother with one.
Comment 7 Joe Orton 2007-08-22 04:59:44 EDT
Thanks for the review!

(In reply to comment #6)
> 1. Does this package need %makeinstall?

Yes, the Makefile doesn't support DESTDIR.

> 2. The URL in the Source line looks wrong...

Fixed in -3 at above location.

> 3. By no means a blocker, but there's no dist tag.

This is by intent, I see no need to use the dist tag on the devel branch for any
package.  (particularly so for a package like this, as you say, which will never
exist in any branch)
Comment 8 Kevin Fenzi 2007-08-22 12:06:52 EDT
ok. Too bad about %makeinstall. ;( 

I see no further blockers here, so this package is APPROVED. 
Don't forget to close this once it's been imported and built. 
Comment 9 Joe Orton 2007-08-22 12:13:59 EDT
New Package CVS Request
=======================
Package Name: compat-expat1
Short Description: Library compatibility package for expat 1
Owners: jorton
Branches: devel
InitialCC: 
Commits by cvsextras: no
Comment 10 Joe Orton 2007-08-22 12:15:05 EDT
New Package CVS Request
=======================
Package Name: compat-expat1
Short Description: Library compatibility package for expat 1
Owners: jorton
Branches: devel
InitialCC: 
Commits by cvsextras: no

[third time lucky with the correct flag waving]
Comment 11 Joe Orton 2007-08-22 16:49:32 EDT
Thanks a lot - now who do I bribe to review compat-neon025! :)

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