[Bug 848388] Review Request: liblognorm - Tool to normalize log data

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

 



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

Michael Schwendt <mschwendt@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mschwendt@xxxxxxxxx

--- Comment #4 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
> Release:	2

Not using %dist during review is okay, but be aware of future trouble when
building the package for multiple dists.


> Summary:	Tool to normalize log data

The library is not really "a tool". The pkgconfig file contains a good
description that could be used for a better %summary:

    Description: fast samples-based log normalization library


* The pkgconfig file as is won't work flawlessly. It contains

    Libs: -L${libdir} -llognorm -lee -lestr

which means a "Requires" dependency is missing in the file, or else one could
not link successfully. However, explicitly relinking with shared libee and
libestr should not be necessary when linking with liblognorm, so these two
linker options should be dropped.

Some of liblognorm's header files access libee/libestr headers, so clearly a
"Requires: libee-devel libestr-devel" is needed in the liblognorm-devel
package. Note that adding a dependency to the .pc file would result in
automatic RPM dependencies.



> %package devel
> Requires:	%{name} = %{version}-%{release}

> %package utils
> Requires:	%{name} = %{version}-%{release}

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


> %description utils
> The normalizer is the core of liblognorm, it's utility for normalizing
> log files.

s/it's/its/ ?


> %build
> make

"V=1 make" for more verbose output (compiler and linker options) in build log
would be nice. Plus (albeit this is just a small source project): 
https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make


> %install
> make install -p DESTDIR=%{buildroot}

Option -p is very likely a mistake here as it is passed to "make" and is short
for "--print-data-base". Assumably, what you actually want is:

  make install INSTALL="install -p"  DESTDIR=%{buildroot}


> $ rpmls -p liblognorm-utils-0.3.4-2.t1.x86_64.rpm 
> -rwxr-xr-x  /usr/bin/normalizer

A very generic name, not in a special namespace, and almost caused a conflict
already:

  # repoquery --whatprovides /usr/bin/normalize
  normalize-0:0.7.7-6.fc17.x86_64

Upstream is highly encouraged to try to avoid using such generic names.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



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