Note: This is a re-review. xfce4-modemlights-plugin hasn't been ported to Xfce
4.4 before, so I orphaned my Xfce 4.2 package after FC-5. The original review
was bug #173548

I'd be happy to review this. Look for a full review in a few. 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPLv2)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
aeab49c57bdaeb12a73486515aae50cd  xfce4-modemlights-plugin-0.1.3.99.tar.bz2
aeab49c57bdaeb12a73486515aae50cd  xfce4-modemlights-plugin-0.1.3.99.tar.bz2.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Spec handles locales/find_lang
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version

Issues:

1. Wouldn't a better URL be:
http://goodies.xfce.org/projects/panel-plugins/xfce4-modemlights-plugin

The Source0 URL seems to be:
http://goodies.xfce.org/releases/xfce4-modemlights-plugin/xfce4-modemlights-plugin-0.1.3.99.tar.bz2

2. rpmlint says:

xfce4-modemlights-plugin.src:65: W: macro-in-%changelog defattr

Might need a %% there to escape that word from macro expansion.

3. Should this 'Requires: ppp' ? Or is it useful without ppp?

(In reply to comment #3)
> 
> Issues:
> 
> 1. Wouldn't a better URL be:
> http://goodies.xfce.org/projects/panel-plugins/xfce4-modemlights-plugin
> 
> The Source0 URL seems to be:
>
http://goodies.xfce.org/releases/xfce4-modemlights-plugin/xfce4-modemlights-plugin-0.1.3.99.tar.bz2
> 
> 2. rpmlint says:
> 
> xfce4-modemlights-plugin.src:65: W: macro-in-%changelog defattr
> 
> Might need a %% there to escape that word from macro expansion.

All valid points, seems like I missed some errors when I reworked the spec.

> 
> 3. Should this 'Requires: ppp' ? Or is it useful without ppp?
> 
I have patched the plugin to use ifup/ifdown by default, so theoretically it can
be used for ethernet or wireless devices too, but I guess no one will do that.
So I require ppp now.

SPEC:
http://home.arcor.de/christoph.wickert/fedora/review/xfce4-modemlights-plugin.spec
SRPM:
http://home.arcor.de/christoph.wickert/fedora/review/xfce4-modemlights-plugin-0.1.3.99-2.fc9.src.rpm

Looks all good. This package is APPROVED. 

I will need to remove the Provides in xfce4-panel before it can come in... 
can wait and do that with the upcoming 4.4.2 release, or before if you prefer. 

I'll wait until your Xfce 4.4.2 panel arrives.

Package Change Request
======================
Package Name: xfce4-modemlights-plugin
New Branches: FC-6 F-7 F-8
Updated Description: Modemlights for the Xfce panel
("Xfce" was upper case before)

cvs done... except for FC-6. We are no longer allowing FC-6 branches. ;( 

Ok, I see, but can't we make an exception for this package? I think it would
really be bad to have modemlights in FC <= 5, not in FC6 and then in F >= 7 again.

Doesn't look like an exception is going to happen. ;( 

I guess this will just be missing in FC-6. Sorry. 

Package is now in the repo, at least for > F-7. Closing.

Package Change Request
======================
Package Name:  xfce4-modemlights-plugin
New Branches: epel7
Owners: cwickert
InitialCC: nonamedotc

Git done (by process-git-requests).