Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 192546 - Review Request: gnubiff
Summary: Review Request: gnubiff
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Brian Pepple
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-20 11:36 UTC by Damien Durand
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-05-24 04:11:53 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Mock build log failure (deleted)
2006-05-21 22:19 UTC, Brian Pepple
no flags Details

Description Damien Durand 2006-05-20 11:36:25 UTC
Spec URL: http://glive.tuxfamily.org/fedora/gnubiff/gnubiff.spec
SRPM URL: http://glive.tuxfamily.org/fedora/gnubiff/gnubiff-2.2.0-1.fc6.src.rpm
Description: Gnubiff is a mail notification program that periodically checks 
for mail and displays headers when new mail has arrived

Comment 1 Aurelien Bompard 2006-05-20 16:48:49 UTC
Brian, are you reviewing this package ? Anyway here's my take at it (can't harm
to have more people for review)

Needs work:
* BuildRequires: gettext is missing (required by the %find_lang macro)
* /usr/share/info/dir is already owned by info, don't own it (put 
  %{_datadir}/info/*.info.gz in %files for example)
* Scriptlets: missing "install-info" in %post and %preun (in the wiki:
  ScriptletSnippets)

Minor:
* Version and Source1 are not properly lined-up (tabs instead of spaces)
* Duplicate BuildRequires: gtk2-devel (by libglade2-devel)
* At the end of ./configure, there is "Gnome support: no". Is that what you want
? From http://gnubiff.sourceforge.net, GNOME support could be useful. Perhaps a
missing "BuildRequires: gnome-panel-devel" only ?


Comment 2 Damien Durand 2006-05-21 21:31:02 UTC
Ok, i've made the changes.
Source : http://glive.tuxfamily.org/fedora/gnubiff/


Comment 3 Brian Pepple 2006-05-21 22:19:03 UTC
Created attachment 129786 [details]
Mock build log failure

Your package fails to build in Mock.  Also, you can drop the BR on
gettext-devel, since the default build environment in Mock installs gettext.

Comment 4 Damien Durand 2006-05-22 20:31:03 UTC
Change available at http://glive.tuxfamily.org/fedora/gnubiff/

Comment 5 Aurelien Bompard 2006-05-23 11:53:08 UTC
Everything looks OK, but how about enabling GNOME support ? It looks like it can
be embedded in the panel this way. It'd be a nice feature to have IMHO.

By the way, version 2.2.1 is out.


Comment 6 Brian Pepple 2006-05-23 14:06:46 UTC
(In reply to comment #5)
> Everything looks OK, but how about enabling GNOME support ? It looks like it can
> be embedded in the panel this way. It'd be a nice feature to have IMHO.
> 
> By the way, version 2.2.1 is out.
> 

In addition to the GNOME support, it looks like it also has some SSL/crypto
support that would be worthwile to enable.



Comment 7 Damien Durand 2006-05-23 17:11:43 UTC
- Upgrade to 2.2.1
- Add --prefix='pkg-config libpanelapplet-2.0 openssl --variable=prefix in %con$
- Add gnome-panel-devel, openssl-devel in BuildRequires

Changes available: http://glive.tuxfamily.org/fedora/gnubiff/

Comment 8 Brian Pepple 2006-05-23 19:26:58 UTC
MD5Sums:
8d2ef679f42e7a593dc88b750d0cca4c  gnubiff-2.2.1.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* All directories are owned by this or other packages
* All necessary BuildRequires listed.
* All desired features are enabled
* Package builds in Mock.
* Package installs and uninstalls cleanly on FC5.
* rpmlint produces no error.

Bad:
* Don't pass '--prefix=`pkg-config libpanelapplet-2.0 openssl
--variable=prefix`' to the %configure macro.  It's not needed.

Minor:
* In the file section, '%{_datadir}/info' should be '%{_infodir}'.  Refer to
http://fedoraproject.org/wiki/Extras/RPMMacros
* Unnecessary documentation: ABOUT-NLS & Changelog.  The first is a generic
build tools file, and the second is duplicate information that is included in
the NEWS file.

Once these items are fixed, considered this approved.

+1 Approve

Comment 9 Damien Durand 2006-05-24 04:11:53 UTC
- Remove --prefix='pkg-config libpanelapplet-2.0 openssl --variable=prefix OK
- Remove ABOUT-NLS & Changelog in %file section OK
- Fixing %{_datadir}/info to {_infodir} OK

Package imported in Extras



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