Bug 225634 - Merge Review: cadaver
Merge Review: cadaver
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Saou
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 12:48 EST by Nobody's working on this, feel free to take it
Modified: 2012-08-20 08:12 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-25 05:34:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
matthias: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 12:48:51 EST
Fedora Merge Review: cadaver

http://cvs.fedora.redhat.com/viewcvs/devel/cadaver/
Initial Owner: jorton@redhat.com
Comment 1 Matthias Saou 2007-09-01 14:01:41 EDT
The current package looks already quite good. A few minor possible spec file
changes :
- Remove the zero epoch from the neon-devel BR
- Include a quick comment as to why pie is needed
- Change %defattr(-,root,root) to the more usual %defattr(-,root,root,-)
  (pretty useless, though, since the package doesn't include directories)
- Move the %doc line up so that it isn't in between two normal lines

I'll test a build and post the results.
Comment 2 Matthias Saou 2007-09-12 07:23:44 EDT
Test build went fine, resulting binary is fine.
You could remove the obsolete "cadaver-0.22.5.tar.gz.asc" from CVS, though.
Comment 3 Joe Orton 2007-09-13 02:34:26 EDT
Thanks for the review!

> - Remove the zero epoch from the neon-devel BR

Fixed.

> - Include a quick comment as to why pie is needed

It's not "needed", it's a feature, I don't think that explaining what -pie is in
every spec file which uses it is really a good idea.

> - Change %defattr(-,root,root) to the more usual %defattr(-,root,root,-)
>   (pretty useless, though, since the package doesn't include directories)

Fixed.

> - Move the %doc line up so that it isn't in between two normal lines

And fixed.  In -2.  Also fixed the use of %makeinstall, which is now unnecessary
in 0.23.0.
Comment 4 Matthias Saou 2007-09-24 09:52:20 EDT
Good catch, the %makinstall, I've often missed it in my own spec files :-/

Everything looks good, except maybe the NLS : Passing --enable-nls doesn't
change anything, and configure always reports "Internationalization:  Not
built". I'm not sure why since a quick look seems to indicate that the messages
about having NLS enabled should lead to "Internationalization" being enabled.

Anyway, this isn't vital, and not review specific, so feel free to have a look
and close the bug, but I'll fedora-review + anyway.
Comment 5 Joe Orton 2007-09-25 05:34:08 EDT
Ah, the "Not built" thing is a cosmetic configure script bug, I've fixed it
upstream.  --enable-nls is in fact the default but being explicit here is good
since the upstream default has changed over time here.

Thanks again for the review.
Comment 6 Steve Traylen 2012-08-20 05:11:10 EDT
Package Change Request
======================
Package Name: cadaver
New Branches: el6
Owners: stevetraylen

See #739496 which was the request for me to take EPEL ownership.
Comment 7 Jon Ciesla 2012-08-20 08:12:16 EDT
Git done (by process-git-requests).

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