Bug 191507 - Review Request: wifi-radar
Review Request: wifi-radar
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-12 12:03 EDT by Ian Pilcher
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-21 13:46:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Ian Pilcher 2006-05-12 12:03:56 EDT
Spec URL: http://home.comcast.net/~i.pilcher/wifi-radar.spec
SRPM URL: http://home.comcast.net/~i.pilcher/wifi-radar-1.9.6-1.src.rpm
Description: A straightforward utility for managing wireless connections
Comment 1 Tom "spot" Callaway 2006-05-26 10:46:07 EDT
There are a few minor issues with this package:

- The package needs to properly handle its desktop file: 
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

Don't forget to include BuildRequires: desktop-file-utils

- rpmlint throws some errors:
W: wifi-radar conffile-without-noreplace-flag /etc/wifi-radar/wifi-radar.conf

Might not be a bad idea to set this as %config(missingok,noreplace) to keep some
other package from stepping on this config file.

E: wifi-radar non-readable /etc/wifi-radar/wifi-radar.conf 0600

I'm pretty sure you didn't intend this. Or did you?

E: wifi-radar non-standard-dir-perm /etc/wifi-radar 0700

Also a weird permission set for the config directory.

E: wifi-radar non-standard-dir-perm /usr/share/doc/wifi-radar-1.9.6 0644

I don't see the reason to deviate from the standard %doc permission set, even
though 644 is valid.

Good items:

- package meets naming guidelines
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR (except for desktop-file-utils)
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- %clean ok
- macro use consistent
- code, not content
- no need for documentation subpackage
- nothing in %doc affects runtime 

Show me a new spec with the minor blockers listed above resolved and I'll
sponsor/approve.
Comment 2 Ian Pilcher 2006-06-01 16:24:52 EDT
(In reply to comment #1)
> There are a few minor issues with this package:

Hmm.  rpmlint appears to give useful output when run against an installed
package, but not when run against an SRPM file.  Interesting.

> - The package needs to properly handle its desktop file: 
> http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

Done, but...

This confuses me.  From what I can tell, the desktop-file-install command
on the Wiki page simple prepends "fedora-" to the name of the desktop file
and adds the "X-Fedora" category.  What's the reason that a package can't
simply supply a desktop file that already meets the naming/category
requirements?

> Don't forget to include BuildRequires: desktop-file-utils

Done.

> W: wifi-radar conffile-without-noreplace-flag /etc/wifi-radar/wifi-radar.conf
> 
> Might not be a bad idea to set this as %config(missingok,noreplace) to keep some
> other package from stepping on this config file.

Done.

> E: wifi-radar non-readable /etc/wifi-radar/wifi-radar.conf 0600
> 
> I'm pretty sure you didn't intend this. Or did you?

WEP keys go in the config file, so I don't want it to be world-readable.
(Although, anyone who can run the application will be able to read them.)

> E: wifi-radar non-standard-dir-perm /etc/wifi-radar 0700
> 
> Also a weird permission set for the config directory.

If root creates a brand new config file (with a text editor, for example) it
will be 0644.  Making the directory 0700 keeps this from exposing any WEP
keys.

> E: wifi-radar non-standard-dir-perm /usr/share/doc/wifi-radar-1.9.6 0644

Fixed.

> I don't see the reason to deviate from the standard %doc permission set, even
> though 644 is valid.

Not really useful for a directory, though.  :-)

> Show me a new spec with the minor blockers listed above resolved and I'll
> sponsor/approve.

Updated SPEC file and SRPM at:

  http://home.comcast.net/~i.pilcher/wifi-radar.spec
  http://home.comcast.net/~i.pilcher/wifi-radar-1.9.6-2.src.rpm
Comment 3 Ville Skyttä 2006-06-01 16:40:23 EDT
> Hmm.  rpmlint appears to give useful output when run against an installed
> package, but not when run against an SRPM file.  Interesting.

Everything cannot be checked from the SRPM alone.  And some checks make sense
only when run against an installed package, so checking an uninstalled binary
RPM won't necessarily reveal everything either.  The best check coverage is
achieved by checking the SRPM and all binary packages resulting from it after
*installing* them.
Comment 4 Christian Iseli 2006-12-30 19:10:28 EST
(I assume spot did accept this package...)

Properly block FE-ACCEPT

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