Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: kismet -- A WLAN detector, sniffer and IDS https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=165314 bugzilla@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- CC|fedora-extras- | |list@xxxxxxxxxx | CC| |fedora-package- | |review@xxxxxxxxxx ------- Additional Comments From j.w.r.degoede@xxxxxx 2006-04-22 09:22 EST ------- Once again apologies for my slowness I was really swamped with work at work lately so much I've even been working my weekends, anyways here's a full review as promised: MUST: ===== * rpmlint output is: W: kismet conffile-without-noreplace-flag /etc/cron.daily/tmpwatch.kismet E: kismet non-standard-gid /etc/kismet kismet E: kismet non-standard-dir-perm /etc/kismet 0750 E: kismet non-standard-gid /var/lib/kismet kismet E: kismet non-standard-dir-perm /var/lib/kismet 0770 E: kismet executable-marked-as-config-file /etc/cron.daily/tmpwatch.kismet E: kismet non-standard-gid /var/log/kismet kismet E: kismet non-standard-dir-perm /var/log/kismet 0730 W: kismet log-files-without-logrotate /var/log/kismet W: kismet-debuginfo dangling-relative-symlink /usr/src/debug/kismet-2005-08-R1/libpcap-0.9.1-kis/bpf_filter.c ./bpf/net/bpf_filter.c Most of these are OK / have a good reason, so they are ok, it would be nice if you could fix the last one though, but that is not a blocker. * Package and spec file named appropriately * Packaged according to packaging guidelines * License (GPL) ok, license file included * spec file is legible and in Am. English. * Source matches upstream * Compiles and builds on devel-x86_64 * BR: ok (see below) * No locales * No shared libraries * Not relocatable * Package owns / or requires all dirs (with some strangeness see Must fix below) * No duplicate files & Permissions ok * %clean & macro usage OK * Contains code only * %doc does not affect runtime, and isn't large enough to warrent a sub package * no -devel package needed, no libs / .la files. * no gui -> no .desktop file required MUST fix: ========= *Summary must not start with "A ...." drop the "A " . *These: Requires(pre): %crontabdir Requires(postun): %crontabdir Seem rather odd neither script does anything with cron, the packages does install files in %crontabdir and does not own them, so the package should plain require %crontabdir or better maybe just own it since you made the other related requires (missingok). Should fix: =========== *Redundant BR: ImageMagick-devel already requires libtiff-devel and libjpeg-devel, please remove those from the BR-s *This comment: # set our 'kismet' user, disable GPS and log into ~kismet/logs by Is no longer correct as logs now go to /var/log/kismet *These compiler warnings look really suspicious: gpsmap.cc:868: warning: comparison is always true due to limited range of data gpsmap.cc:876: warning: comparison is always true due to limited range of data Nice to have: ============= * Upstream has a much newer version, it would be nice to upgrade to that version. Thats it, if you fix the 2 MUST fix items and post a new spec / srpm I'll aprove it ASAP. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.