Bug 225634 - Merge Review: cadaver
Summary: Merge Review: cadaver
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Matthias Saou
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 17:48 UTC by Nobody's working on this, feel free to take it
Modified: 2012-08-20 12:12 UTC (History)
2 users (show)

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


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 17:48:51 UTC
Fedora Merge Review: cadaver

http://cvs.fedora.redhat.com/viewcvs/devel/cadaver/
Initial Owner: jorton@redhat.com

Comment 1 Matthias Saou 2007-09-01 18:01:41 UTC
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 11:23:44 UTC
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 06:34:26 UTC
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 13:52:20 UTC
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 09:34:08 UTC
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 09:11:10 UTC
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 Gwyn Ciesla 2012-08-20 12:12:16 UTC
Git done (by process-git-requests).


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