Bug 960199 - Review Request: libyui-ncurses - Character Based User Interface for libyui
Summary: Review Request: libyui-ncurses - Character Based User Interface for libyui
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 959926
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-06 16:43 UTC by Björn 'besser82' Esser
Modified: 2014-03-18 11:59 UTC (History)
1 user (show)

Fixed In Version: libyui-ncurses-2.43.9-3.fc19
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-05-16 12:50:25 UTC
Type: ---
Embargoed:
bugs.michael: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Björn 'besser82' Esser 2013-05-06 16:43:32 UTC
Spec URL: http://besser82.fedorapeople.org/libyui/SPECS/libyui-ncurses.spec
SRPM URL: http://besser82.fedorapeople.org/libyui/SRPMS/libyui-ncurses-2.43.8-1.fc18.src.rpm
Description: This package contains the character based (ncurses) user interface component for libyui

This package depends on libyui which has it's review request in #959926 and also waits for a sponsor, so testing with koji was not possible at the moment.

Fedora Account System Username: besser82

Hello!

This is one my first packages I want to contribute to Fedora, so I need a sponsor. I am in good contact to upstream, esp. to the lead-maintainer and have direct-push access to the libyui github-repos, too.

BR,
  Björn Esser

Comment 1 Björn 'besser82' Esser 2013-05-09 08:44:21 UTC
Updated to V2.43.8

You can find SPEC/SRPM here: http://besser82.fedorapeople.org/pkg/libyui-ncurses/

Or clone my git-repo from besser82:public_git/libyui.git and checkout the libyui-ncurses branch.

BR,
  Björn Esser

Comment 2 Björn 'besser82' Esser 2013-05-13 18:59:01 UTC
Spec URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses.spec
SRPM URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses-2.43.8-1.fc18.src.rpm

Adapted Spec/SRPM urls for tracking inside "fedora-review" tool.

Comment 3 Michael Schwendt 2013-05-15 10:07:45 UTC
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.

Comment 4 Björn 'besser82' Esser 2013-05-15 10:31:19 UTC
Spec URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses.spec
SRPM URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses-2.43.8-2.fc18.src.rpm

Updated the pkg according your advice. Thanks for reviewing.

%changelog
* Wed May 15 2013 Björn Esser <bjoern.esser> - 2.43.8-2
- fixup as suggested in https://bugzilla.redhat.com/show_bug.cgi?id=960199#c3
- exchanged {parname} with {libname}

* Mon May 13 2013 Björn Esser <bjoern.esser> - 2.43.8-1
- Initial RPM release.

Comment 5 Björn 'besser82' Esser 2013-05-15 11:40:19 UTC
Spec URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses.spec
SRPM URL: http://besser82.fedorapeople.org/pkg/libyui-ncurses/libyui-ncurses-2.43.9-1.fc18.src.rpm

New upstream version. Thanks for your review.

%changelog
* Wed May 15 2013 Björn Esser <bjoern.esser> - 2.43.9-1
- new upstream version
- adjusted %{libname}-devel min-version
- added needed bootstrap to prep

* Wed May 15 2013 Björn Esser <bjoern.esser> - 2.43.8-2
- fixup as suggested in https://bugzilla.redhat.com/show_bug.cgi?id=960199#c3
- exchanged {parname} with {libname}

* Mon May 13 2013 Björn Esser <bjoern.esser> - 2.43.8-1
- Initial RPM release.

Comment 6 Michael Schwendt 2013-05-15 17:47:15 UTC
> %package devel
>
> Requires:	ncurses

ncurses-devel%{?_isa}  of course. Guess I managed to confuse you with the suggestion to get rid of some macros. Ha! :)


Beyond that, save yourself another update in this bugzilla review request, since it'll be more convenient to fix it in Fedora pkg git:

APPROVED

Comment 7 Björn 'besser82' Esser 2013-05-15 18:34:41 UTC
New Package SCM Request
=======================
Package Name: libyui-ncurses
Short Description: Character Based User Interface for libyui
Owners: besser82
Branches: f19

Comment 8 Gwyn Ciesla 2013-05-15 19:03:33 UTC
Git done (by process-git-requests).

Comment 9 Björn 'besser82' Esser 2013-05-16 09:57:15 UTC
Package Change Request
======================
Package Name: libyui-ncurses
New Branches: f18
Owners: besser82

Comment 10 Gwyn Ciesla 2013-05-16 12:21:01 UTC
Git done (by process-git-requests).

Comment 11 Fedora Update System 2013-05-16 13:57:41 UTC
libyui-ncurses-2.43.9-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/libyui-ncurses-2.43.9-3.fc18

Comment 12 Fedora Update System 2013-05-16 13:58:24 UTC
libyui-ncurses-2.43.9-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/libyui-ncurses-2.43.9-3.fc19

Comment 13 Fedora Update System 2013-05-26 02:57:17 UTC
libyui-ncurses-2.43.9-3.fc18 has been pushed to the Fedora 18 stable repository.

Comment 14 Fedora Update System 2013-05-26 03:41:23 UTC
libyui-ncurses-2.43.9-3.fc19 has been pushed to the Fedora 19 stable repository.

Comment 15 Björn 'besser82' Esser 2014-03-18 11:44:43 UTC
Package Change Request
======================
Package Name: libyui-ncurses
New Branches: el6
Owners: besser82

Want to build on el6, too.

Comment 16 Gwyn Ciesla 2014-03-18 11:59:40 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.