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 187569 - Review Request: xfce4-mailwatch-plugin
Summary: Review Request: xfce4-mailwatch-plugin
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-04-01 00:10 UTC by Christoph Wickert
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-05-05 19:52:57 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Christoph Wickert 2006-04-01 00:10:26 UTC
Spec Name or Url: http://home.arcor.de/christoph.wickert/fedora/extras-review/SPECS/xfce4-mailwatch-plugin.spec
SRPM Name or Url: http://home.arcor.de/christoph.wickert/fedora/extras-review/SRPMS/xfce4-mailwatch-plugin-1.0.0-1.fc5.src.rpm
Description: Mailwatch is a plugin for the Xfce 4 panel. It is intended to replace the current (4.0, 4.2) mail checker plugin in Xfce 4.4. It supports IMAP and POP, local mailboxes in Mbox, Maildir and MH-Maildir format as well as Gmail.

Comment 1 Kevin Fenzi 2006-04-10 01:07:25 UTC
A review:

OK - Rpmlint output.
OK - Package name.
OK - Spec file name matches.
OK - Package guidelines.
OK - Licsense. (GPL)
OK - License field matches in spec.
OK - License included in files
OK - Spec in american english
OK - Spec legible
OK - Md5sum of source from upstream
e31d32b08f82e24e730831641cdd65f0  xfce4-mailwatch-plugin-1.0.0.tar.bz2
e31d32b08f82e24e730831641cdd65f0  xfce4-mailwatch-plugin-1.0.0.tar.bz2.1
OK - Compiles and builds on one arch at least.
See below - All required buildrequires included?
OK - Locale handling/find_lang.
OK - Owns all directories it creates.
OK - No duplicate files in %files listing.
OK - Permissions on files correct.
OK - Clean section correct.
OK - Macros consistant.
OK - Code not content.
OK - No .la files.
See Below - Doesn't own any files/dirs that are already owned by others.

Items needing attention:

1. The summary has "Summary:        Quicklauncher plugin for the Xfce panel"
Shouldn't that be "Mail Watcher plugin for the Xfce panel"?

2. Some of the dirs that this package owns are owned by lots of
other packages. In particular:

/usr/share/icons/hicolor
/usr/share/icons/hicolor/48x48
/usr/share/icons/hicolor/48x48/apps
/usr/share/icons/hicolor/scalable
/usr/share/icons/hicolor/scalable/apps
/usr/share/xfce4/doc/C
/usr/share/xfce4/doc/C/images

This package shouldn't also need to own those I wouldn't think.
Perhaps require hicolor-icon-theme and xfce4-panel to make sure those
dir dependencies are met.

3. Doesn't build in mock. Looks like it might be missing:
BuildRequires: imake libXt-devel


Comment 2 Christoph Wickert 2006-04-10 23:48:24 UTC
Hi Kevin, thanks for your review.

(In reply to comment #1)

> Items needing attention:
> 
> 1. The summary has "Summary:        Quicklauncher plugin for the Xfce panel"
> Shouldn't that be "Mail Watcher plugin for the Xfce panel"?

D'oh! Fixed.

> 2. Some of the dirs that this package owns are owned by lots of
> other packages. 
> [...]
> Perhaps require hicolor-icon-theme and xfce4-panel to make sure those
> dir dependencies are met.

Oh my god - I just realized that _all_ my panel plugins lack an explicit
requirement on xfce4-panel! Going to fix this ASAP.

With xfce4-panel we also have the dirs, at least the doc dirs:
/usr/share/xfce4/doc
/usr/share/xfce4/doc/C
/usr/share/xfce4/doc/C/images

No need to add hicolor-icon-theme since it is required by gtk2.

> 3. Doesn't build in mock. Looks like it might be missing:
> BuildRequires: imake libXt-devel

imake is not required and libXt-devel BR is already included inside the 
"%if "%fedora" > "4""-statement. Unfortunately this is not picked up by
mock on core 5 since it does not install the buildsys-macros-rpm. See 

https://www.redhat.com/archives/fedora-extras-list/2006-April/msg00582.html

After fixing builtroots.xml the packge builds fine (again, the previous packages
are mock-builds too) in mock. AFAIK all builthosts install the macros correctly,
at least according to the logs of some of my latest builds where I'm using
selective specfiles, too.

Updated Spec
http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-mailwatch-plugin.spec
New SRPM:
http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-mailwatch-plugin-1.0.0-2.fc5.src.rpm

%changelog
* Mon Apr 10 2006 Christoph Wickert <fedora wickert at arcor de> - 1.0.0-2
- Fix description.
- Fix files section.
- Require xfce4-panel.

Comment 3 Kevin Fenzi 2006-04-12 00:19:27 UTC
Ah ha. That macros thing was confusing me on some other mock builds too. 

Once Xfce-4.4 comes out we will have to do something as you mentioned that this
plugin is part of xfce4-panel. Perhaps make this package 
Requires: xfce4-panel < 4.4 ?

With that macro fix, builds fine under mock and all the blockers look to be
fixed up, so this package is APPROVED. 

Comment 4 Christoph Wickert 2006-05-05 19:52:57 UTC
Thanks fpr the review, Kevin. 

Imported and build for FC-4 and FC-5. Rawhide is hosed ATM. Closing.


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