[Bug 435018] Review Request: clipper - crystallographic object oriented library

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=435018





--- Comment #16 from Tim Fenn <fenn at stanford.edu>  2008-11-13 15:49:56 EDT ---
(In reply to comment #15)
> For -17:
> 
> * License
>   - Well, the description of the license in the source codes seems
>     somewhat confusing, however the license tag can be safe
>     with "LGPLv2+".
> 

OK.

> * Source1
>   - Is this source created by yourself or can it be downloaded
>     somewhere? (If some URL exists, write the URL like
>     Source0)
> 

Created by me - I've submitted it upstream as well.

> * Dependency for -devel subpackage
>   - Installed clipper.pc contains:
> --------------------------------------------------------
>      8  Requires: mmdb gpp4
>     10  Libs: -L/usr/lib -lclipper -lsrfftw -lsfftw -lm 
> --------------------------------------------------------
>     This means that -devel subpackage must have
>     "Requires: mmdb-devel gpp4-devel fftw2-devel", however
>     - Would you check if "Requires: gpp4" and
>       "Libs: -lstfftw -lsfftw" are really needed?
>       Installed clipper-devel header files do not seem to
>       depend on any header files in gpp4-devel or fftw2-devel",
>       so I guess these Requires and Libs can be removed
> 

several of the functions under the ccp4/ folder are wrapped C++ calls to gpp4
functions, and functions in core/fftmap.h contain wrapped fftw calls.  I've
added the Requires to -devel.

> 
> * %setup
>   - I guess %setup can be replaced by
> --------------------------------------------------------
> %prep
> %setup -q -c -a 1
> --------------------------------------------------------
> 

done.

> ! Timestamps
>   - Not a blocker, however for packages using "install-sh" to
>     install files, 
> --------------------------------------------------------
> make install DESTDIR=$RPM_BUILD_ROOT CPPROG="cp -p"
> --------------------------------------------------------
>     to keep timestamps on installed files
> 

done.

> * %defattr
>   - Now we recommend %defattr(-,root,root,-)
> 

oops, fixed.

> * Documents
>   - The file "INSTALL" is usually for people who wants to compile/
>     install the package by himself and not needed for rpm users.
> 

removed.

> * Directory ownership issue
>   - %_includedir/clipper is not owned by any packages
>     ref:
>    

Ah, thanks!  fixed.

> 
> * rpmlint
> --------------------------------------------------------
> clipper.i386: E: zero-length /usr/share/doc/clipper-2.0/ChangeLog
> clipper-devel.i386: W: wrong-file-end-of-line-encoding
> /usr/share/doc/clipper-devel-2.0/dox/develop.dox
> clipper-devel.i386: W: wrong-file-end-of-line-encoding
> /usr/share/doc/clipper-devel-2.0/dox/coordtypes.dox
> clipper-devel.i386: W: spurious-executable-perm
> /usr/share/doc/clipper-devel-2.0/dox/wheretolook.dox
> --------------------------------------------------------
>   - Including zero size "ChangeLog" seems meaningless
>   - Please fix CRLF (Windows-like) end-of-line endcodings.
>   - Usually files included as %doc must have 0644 permission

removed the changelog file, fixed CRLF foo and file perms.

new spec: http://www.stanford.edu/~fenn/packs/clipper.spec
new srpm: http://www.stanford.edu/~fenn/packs/clipper-2.0-18.f8.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.