Bug 177104 - Review Request: abook - Text-based addressbook program for mutt
Review Request: abook - Text-based addressbook program for mutt
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2006-01-06 07:32 EST by Dominik 'Rathann' Mierzejewski
Modified: 2012-09-16 17:47 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-08-16 14:21:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Dominik 'Rathann' Mierzejewski 2006-01-06 07:32:15 EST
Spec: http://rpm.greysector.net/extras/abook.spec
SRPM: http://rpm.greysector.net/extras/abook-0.5.5-1.src.rpm
Abook is a small and powerful text-based addressbook program
designed for use with the mutt mail client.
Comment 1 Wart 2006-01-29 19:19:48 EST
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:

* 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-04 19:43:00 EST

* 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-04 19:44:35 EST
> * 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 16:19:50 EDT
Fixed. http://rpm.greysector.net/extras/abook-0.5.5-2.src.rpm
Comment 5 Michael Schwendt 2006-04-04 17:52:09 EDT
It's still missing to empty the buildroot at beginning of %install.
Fix that after CVS import.

Comment 6 Michael Schwendt 2006-05-29 07:21:01 EDT
Dropping FE-NEEDSPONSOR. Tom Callaway offered sponsorship in bug 177235
(and bugzilla change-several-bugs-at-once feature requires me to add/edit
Comment 7 Michael Schwendt 2006-05-29 07:24:01 EDT
uhm, bugzilla is broken :(
Comment 8 Michael Schwendt 2006-07-06 05:04:12 EDT
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 03:09:58 EDT
Updated to 0.5.6.

Comment 10 Jason Tibbitts 2006-08-12 20:29:09 EDT
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
* %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 18:24:54 EDT
rpmlint is now quiet; everything looks good to me.

Comment 13 Dominik 'Rathann' Mierzejewski 2006-08-16 14:21:07 EDT
Imported and built for devel, FC5 branch requested.
Comment 14 Dominik 'Rathann' Mierzejewski 2012-09-16 13:08:43 EDT
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 Jon Ciesla 2012-09-16 17:47:53 EDT
Git done (by process-git-requests).

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