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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody@xxxxxxxxxxxxxxxxx    |mtasaka@xxxxxxxxxxxxxxxxxxx
               Flag|                            |fedora-review?




--- Comment #15 from Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx>  2008-11-13 04:35:06 EDT ---
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+".

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

* 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

    ! Note: "Requires: mmdb" is still needed (this means
      that clipper-devel must have "Requires: mmdb-devel")
      because clipper/mmdb/clipper_mmdb.h contains:
--------------------------------------------------------
    50  
    51  #include <mmdb/mmdb_manager.h>
    52  
--------------------------------------------------------

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

! 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

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

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

* Directory ownership issue
  - %_includedir/clipper is not owned by any packages
    ref:
   
https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Wildcarding_Files_inside_a_Created_Directory

* 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

-- 
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.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]