Bug 177104 - Review Request: abook - Text-based addressbook program for mutt
Summary: Review Request: abook - Text-based addressbook program for mutt
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-01-06 12:32 UTC by Dominik 'Rathann' Mierzejewski
Modified: 2012-09-16 21:47 UTC (History)
1 user (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2006-08-16 18:21:07 UTC
Type: ---
Embargoed:
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Dominik 'Rathann' Mierzejewski 2006-01-06 12:32:15 UTC
Spec: http://rpm.greysector.net/extras/abook.spec
SRPM: http://rpm.greysector.net/extras/abook-0.5.5-1.src.rpm
Description:
Abook is a small and powerful text-based addressbook program
designed for use with the mutt mail client.

Comment 1 Wart 2006-01-30 00:19:48 UTC
This isn't a formal review because I can't sponsor you, but here are a couple of
comments on the spec file:

* Consider if you want to use %{?dist} in the release line.  It's optional, but
many packagers find it useful.  This can also be added after importing into CVS.

* Please use the recommended value for BuildRoot per the packaging guidelines:
http://fedoraproject.org/wiki/Packaging/Guidelines

* Don't add the check for "/" in %clean.  Just remove $RPM_BUILD_ROOT with no
checks.  The BuildRoot: setting ensures that it won't be "/".

* rpmlint warnings:
W: abook non-standard-group Networking/Mail
Applications/Internet or Applications/Productivity might be more appropriate.

Comment 2 Michael Schwendt 2006-03-05 00:43:00 UTC
Additionally:

* missing   rm -rf ${RPM_BUILD_ROOT}   as first line of %install

* missing   BuildRequires: gettext

* it defaults to running "lynx" as a textmode web browser,
  when not installed, the error message is not obvious, since it
  only flashes on the screen very briefly -- alternatively:
  Requires: lynx

* don't package in %doc the INSTALL file, since it is irrelevant and
  confusing to users of your package

* prefer   make DESTDIR=${RPM_BUILD_ROOT}   over %makeinstall macro
  unless the latter breaks


Comment 3 Michael Schwendt 2006-03-05 00:44:35 UTC
> * prefer   make DESTDIR=${RPM_BUILD_ROOT}   over %makeinstall macro
>   unless the latter breaks

should read:

* prefer   make DESTDIR=${RPM_BUILD_ROOT}   over %makeinstall macro
  unless the former breaks and the latter works instead


Comment 4 Dominik 'Rathann' Mierzejewski 2006-04-02 20:19:50 UTC
Fixed. http://rpm.greysector.net/extras/abook-0.5.5-2.src.rpm


Comment 5 Michael Schwendt 2006-04-04 21:52:09 UTC
It's still missing to empty the buildroot at beginning of %install.
Fix that after CVS import.

APPROVED


Comment 6 Michael Schwendt 2006-05-29 11:21:01 UTC
Dropping FE-NEEDSPONSOR. Tom Callaway offered sponsorship in bug 177235
(and bugzilla change-several-bugs-at-once feature requires me to add/edit
something).

Comment 7 Michael Schwendt 2006-05-29 11:24:01 UTC
uhm, bugzilla is broken :(

Comment 8 Michael Schwendt 2006-07-06 09:04:12 UTC
Withdrawing my approval from 2006-04-04. Back to FE-NEW.
See bug 177105 comment 14 for background.


Comment 9 Dominik 'Rathann' Mierzejewski 2006-08-01 07:09:58 UTC
Updated to 0.5.6.

http://rpm.greysector.net/extras/abook.spec
http://rpm.greysector.net/extras/abook-0.5.6-1.src.rpm

Comment 10 Jason Tibbitts 2006-08-13 00:29:09 UTC
I'll go ahead and take a look at this.

It builds fine in mock; rpmlint on the SRPM has this to say:

W: abook macro-in-%changelog rlz1
W: abook macro-in-%changelog rlz1
W: abook macro-in-%changelog rlz1
W: abook macro-in-%changelog rlz1
W: abook macro-in-%changelog rlz1
  You just need to double some percent signs.

E: abook no-cleaning-of-buildroot
   You should clean out $RPM_BUILD_ROOT at the beginning of %install.

rpmlint on the built RPM is quiet.

* source files match upstream:
   87d25df96864a7c507a4965e6d1da49d  abook-0.5.6.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* debuginfo package looks complete.
X rpmlint has valid complaints
* final provides and requires are sane:
   abook = 0.5.6-1.fc6
  =
   libncursesw.so.5()(64bit)
   libreadline.so.5()(64bit)
   lynx
* %check is not present; no test suite upstream
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.
* locale files present; fing_lang used appropriately.

Comment 12 Jason Tibbitts 2006-08-15 22:24:54 UTC
rpmlint is now quiet; everything looks good to me.

APPROVED

Comment 13 Dominik 'Rathann' Mierzejewski 2006-08-16 18:21:07 UTC
Imported and built for devel, FC5 branch requested.

Comment 14 Dominik 'Rathann' Mierzejewski 2012-09-16 17:08:43 UTC
Package Change Request
======================
Package Name: abook
New Branches: el6
Owners: rathann

I'd like to maintain the EPEL-6 branch of this package as well.

Comment 15 Gwyn Ciesla 2012-09-16 21:47:53 UTC
Git done (by process-git-requests).


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