Bug 832643 - Review Request: xfce4-kbdleds-plugin - Xfce panel plugin to show the state of keyboard LEDs
Review Request: xfce4-kbdleds-plugin - Xfce panel plugin to show the state of...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-16 05:18 EDT by hannes
Modified: 2012-06-16 15:09 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-16 15:09:17 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description hannes 2012-06-16 05:18:08 EDT
Spec URL: http://hannes.fedorapeople.org/xfce4-kbdleds-plugin.spec
SRPM URL: http://hannes.fedorapeople.org/xfce4-kbdleds-plugin-0.0.6-1.fc17.src.rpm
Description: This panel plugin allows to monitor the state of the keyboard LEDs.
It shows the state of the keys Caps-Lock, Scroll and Num Lock in 
your Xfce panel
This is especially helpful if your computer doesn't have those LEDs.
Fedora Account System Username: hannes

Scratch-Build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4169910

rpmlint output:
rpmlint xfce4-kbdleds-plugin
xfce4-kbdleds-plugin.x86_64: W: spelling-error Summary(en_US) Xfce -> Face
xfce4-kbdleds-plugin.x86_64: E: incorrect-fsf-address /usr/share/doc/xfce4-kbdleds-plugin-0.0.6/COPYING
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

The fsf address thing is caused by a wrong template of Xfce and it is a known problem upstream. I will let the developer of this plugin know about this, so that he could update it.

rpmlint ~/downloads/xfce4-kbdleds-plugin-0.0.6-1.fc17.x86_64.rpm 
xfce4-kbdleds-plugin.x86_64: W: spelling-error Summary(en_US) Xfce -> Face
xfce4-kbdleds-plugin.x86_64: E: incorrect-fsf-address /usr/share/doc/xfce4-kbdleds-plugin-0.0.6/COPYING
1 packages and 0 specfiles checked; 1 errors, 1 warnings.
Comment 1 Kevin Fenzi 2012-06-16 12:00:18 EDT
I'll review this. Look for a review in a few here.
Comment 2 Kevin Fenzi 2012-06-16 12:05:51 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
See below - License
See below - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
db6ad8e3502f3373f087ba2034141552  xfce4-kbdleds-plugin-0.0.6.tar.bz2
db6ad8e3502f3373f087ba2034141552  xfce4-kbdleds-plugin-0.0.6.tar.bz2.orig
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
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. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have sane scriptlets. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. The license here seems to be GPLv2+, not BSD?

2. The Source0 url isn't right. 
Changing: 
%global minor_version 0.0
fixes it. 

3. rpmlint says: 

xfce4-kbdleds-plugin.src: W: invalid-url Source0: http://archive.xfce.org/src/panel-plugins/xfce4-kbdleds-plugin/1.0/xfce4-kbdleds-plugin-0.0.6.tar.bz2 HTTP Error 404: Not Found
(fixed by above change)

xfce4-kbdleds-plugin.i686: W: spelling-error Summary(en_US) Xfce -> Face
xfce4-kbdleds-plugin.i686: E: incorrect-fsf-address /usr/share/doc/xfce4-kbdleds-plugin-0.0.6/COPYING
xfce4-kbdleds-plugin.src: W: spelling-error Summary(en_US) Xfce -> Face
xfce4-kbdleds-plugin-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/xfce4-kbdleds-plugin-0.0.6/panel-plugin/xkbleds.c
xfce4-kbdleds-plugin-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/xfce4-kbdleds-plugin-0.0.6/panel-plugin/kbdleds-dialogs.c
xfce4-kbdleds-plugin-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/xfce4-kbdleds-plugin-0.0.6/panel-plugin/xkbleds.h
xfce4-kbdleds-plugin-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/xfce4-kbdleds-plugin-0.0.6/panel-plugin/kbdleds.c
xfce4-kbdleds-plugin-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/xfce4-kbdleds-plugin-0.0.6/panel-plugin/kbdleds.h
3 packages and 0 specfiles checked; 6 errors, 3 warnings.

The rest can be ignored as you mentioned. 

Provided you fix the sourceurl and license before checkin, this package is APPROVED.
Comment 3 hannes 2012-06-16 12:33:15 EDT
I will fix those issues before I check it in, I wonder why rpmlint on the spec did not find this issue with the Source URL.
Thanks a lot for this quick review!

New Package SCM Request
=======================
Package Name: xfce4-kbdleds-plugin
Short Description: Xfce panel plugin to show the state of keyboard LEDs
Owners: hannes
Branches: f17 f16
InitialCC:
Comment 4 Gwyn Ciesla 2012-06-16 14:42:08 EDT
Git done (by process-git-requests).
Comment 5 hannes 2012-06-16 15:09:17 EDT
Thanks for this flawless review and git setup!

rawhide build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4171104

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