Bug 173090 - Review Request: hpic -- Healpix manipulation binaries and library
Review Request: hpic -- Healpix manipulation binaries and library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ed Hill
David Lawrence
http://cmb.phys.cwru.edu/hpic/
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-11-13 19:34 EST by Matthew Truch
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-11-14 13:50:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Matthew Truch 2005-11-13 19:34:24 EST
Spec Name or Url: http://matt.truch.net/fedora/hpic.spec
SRPM Name or Url: http://matt.truch.net/fedora/hpic-0.52-1.src.rpm
Description: 
hpic is a C and C++ implementation of the healpix pixelization of a sphere
algorithms.  It includes several command line utilities for reading, writing,
and manipulating healpix files, as well as a library so other programs can
do the same.
Comment 1 Ed Hill 2005-11-13 21:06:26 EST
Hi Matt, heres a review:

nits:
 - please remode the INSTALL and COPYING files from %doc since the former
   provides no useful info to an end user and the latter is just another 
   copy of the GPL v2

good:
 + specfile is easy to read--good!
 + source matches upstream
 + license is indeed GPL v2
 + builds on FC-4 with the latest cfitsio-devel
 + no *.la files -- good
 + correct use of devel
 + shared libs look good
 + only warning from rpmlint was "W: hpic-devel no-documentation"
     which can be safely ignored

APPROVED.
Comment 2 Ed Hill 2005-11-13 21:10:23 EST
Please s|remode|remove| above.  ;-)
Comment 3 Matthew Truch 2005-11-13 21:14:12 EST
(In reply to comment #1)

> nits:
>  - please remode the INSTALL and COPYING files from %doc since the former
>    provides no useful info to an end user and the latter is just another 
>    copy of the GPL v2

I'll certainly remove the INSTALL file, but isn't it the inclusion of the
COPYING file required as per the PackageReviewGuidlines (even though it means
people probably have 3 billion copies of the GPL installed):
"MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc."
Comment 4 Ed Hill 2005-11-13 21:29:09 EST
Yes, you're right about the COPYING file.  Please include it and ignore 
my earlier comment.
Comment 5 Adrian Reber 2005-11-14 02:40:37 EST
I just saw the commit log of this import and I think there should be some small
changes to the spec file:

The URL line should be the Source0 line and the URL from the %description should
be moved to the URL line.

I am not sure about this, but I think that the -devel package should require
cfitsio-devel because the header file includes headers from that package.
Comment 6 Matthew Truch 2005-11-14 11:06:08 EST
(In reply to comment #5)
> The URL line should be the Source0 line and the URL from the %description should
> be moved to the URL line.
> 
> I am not sure about this, but I think that the -devel package should require
> cfitsio-devel because the header file includes headers from that package.

I agree on both counts.  These fixes are in 0.52-3, which I'll build shortly.
Comment 7 Michael Schwendt 2005-12-24 12:41:07 EST
> %files devel
> %{_libdir}/libhpic.so

%defattr(..) missing. If rebuilt, files in binary package will be owned
by build user.
Comment 8 Matthew Truch 2005-12-24 13:55:48 EST
Re comment 7(In reply to comment #7)
> > %files devel
> > %{_libdir}/libhpic.so
> 
> %defattr(..) missing. If rebuilt, files in binary package will be owned
> by build user.
> 

You're right.  Fixed in 0.52-5

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