[Bug 489233] Review Request: rmol - C++ Revenue Management Optimisation Library (RMOL)

[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=489233


Michael Schwendt <bugs.michael@xxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugs.michael@xxxxxxx




--- Comment #4 from Michael Schwendt <bugs.michael@xxxxxxx>  2009-03-09 07:33:48 EDT ---
It's likely that I've caught all issues with this package, but overall the
package needs quite a lot of work:


* Run "rpmlint" on the src.rpm and built rpms. Pay extra attention to all
warnings about executable files.


* The %doc file "INSTALL" is irrelevant to RPM package users.


> BuildRequires:  automake, autoconf, libtool

These are not used and therefore not required.


> %configure --enable-static 
>>>
> -rw-r--r--  /usr/lib/librmol.a
> -rwxr-xr-x  /usr/lib/librmol.la

https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries


* The documentation is completely mispackaged for several reasons.

1) In the build.log:

checking for doxygen... no
[...]
  - html-doc .......... : no
[...]
  - doxygen ........... : no

2) The chosen "Group: System Environment/Libraries" for the subpackage is
wrong. Should be "Group: Documentation".

3) You configure  --with-docdir=%{_docdir}/%{name}-%{version}  which conflicts
with the %doc macro, because %{_docdir}/%{name}-%{version} is emptied before
any local %doc files are placed in it. Display the contents of your -html-doc
subpackage to see.

4) There are NamingGUidelines for documentation subpackages:
https://fedoraproject.org/wiki/PackagingNamingGuidelines#Documentation_SubPackages


* There are guidelines with examples for several types of RPM spec scriptlets.
Please update your %post and %preun scriptlets:
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Texinfo


* The -devel subpackage ought to "Requires: pkgconfig" as it places a file in
%_libdir/pkgconfig/


> %{_datadir}/man/man3/%{name}.3.gz
> %{_datadir}/man/man1/%{name}-config.1.gz

Use %{_mandir} instead of %_datadir/man and '*' instead of '.gz' as the
compression method chosen by rpmbuild may change.


> %{_datadir}/info/%{name}-ref.info.gz

* Use %{_infodir} instead of %{_datadir}/info and '*' instead of '.gz' as the
compression method chosen by rpmbuild may change.


* Empty lines between %changelog comments increase readability. There are
guidelines on how to include the package version-release in the %changelog:
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs


* The /usr/bin/rmol-config script does questionable things:

$ rmol-config --libs
-lgsl -lgslcblas -lm -L/usr/lib -lrmol

1) Explicitly linking against libgsl* is not necessary (only for static
builds!). You would need "Requires: gsl-devel" in the -devel package for this
to work at all. And the pkgconfig file does not depend on gsl either, which is
correct.

2) libm typically is linked implicitly by GCC.

3) -L/usr/lib is bad as /usr/lib is standard search-path already, so what this
does is to override user-defined search-path with standard search-path.

$ rmol-config --cflags
-I/usr/include -I/usr/include

Same as 3) above. What this does is to override user's search-path for headers
with standard search-path for headers.

$ rmol-config --cflags-debug
-I/usr/include -I/usr/include

Hmm? Not even -g is set here.


> /usr/include/rmol/config.h

This is an internal header file that is not part of the API and should not be
distributed as it causes bad side-effects.

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