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 191507
Summary: | Review Request: wifi-radar | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ian Pilcher <arequipeno> |
Component: | Package Review | Assignee: | Tom "spot" Callaway <tcallawa> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | ||
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-06-21 17:46:47 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Ian Pilcher
2006-05-12 16:03:56 UTC
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. (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 > 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.
(I assume spot did accept this package...) Properly block FE-ACCEPT |