[Bug 960199] Review Request: libyui-ncurses - Character Based User Interface for libyui

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

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=960199

--- Comment #3 from Michael Schwendt <mschwendt@xxxxxxxxx> ---
Only some minor things:


* Macro fun. ;-)

Not a blocker according to the guidelines, but I need to point out that I find
the growing number of %globals less readable. They will cause you some
headaches eventually. Of course, you're free to use so many macros, it's just
not really beneficial. Example:

  %global uitype ncurses
  %global uiname %{uitype}

  BuildRequires:  %{uitype}-devel

ncurses is unlikely to be renamed, so spelling out its name would be more
clear. In case you want the spec file be suitable for target distributions
other than Fedora, ncurses-devel might even be named ncurses-dev instead, and
then you would need to change the macros here already.

  $ grep uiname *.spec
  %global uiname %{uitype}

An unused macro, it seems, and it's just a redefinition of %uitype anyway.

Using many custom macros in Provides and Requires can lead to trouble easily.
Touching the macro definition in a single place may make the built package
provide or require wrong things. Such an error (or typing mistake) would not be
obvious when viewing the automatically sent git commit diff mail. It could be
something ugly such as an unexpanded macro name. And yes, it's a pitfall other
packagers have run into before.


> %description devel

s/sufficent/sufficient/

rpmlint also finds that when you run it on the binary rpms.


> %post -p /sbin/ldconfig
> 
> %postun -p /sbin/ldconfig

Not needed.

This package doesn't store any shared libs in run-timer linker's search path.
The plugins are stored in private path %_libdir/yui/ and are dlopen()'ed at
run-time via their full path (and including the major library version,
currently ".so.5").


> %files
> %dir %{_libdir}/%{libsuffix}

This directory is included in package %parname (libyui) already. 
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_also_owned_by_a_package_implementing_required_functionality_of_your_package


> %files devel

> %dir %{_libdir}/%{libsuffix}

> %{_includedir}/%{libsuffix}

> %{_datadir}/%{parname}

These dirs are also provided by libyui already. Modern RPM can handle that also
upon removing the packages.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=NigxAmwSZb&a=cc_unsubscribe
_______________________________________________
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]