Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=480870


Bill Nottingham <nott...@redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nob...@fedoraproject.org    |nott...@redhat.com
               Flag|                            |fedora-review?




--- Comment #2 from Bill Nottingham <nott...@redhat.com>  2009-01-21 15:49:29 
EDT ---
MUST items:

- Package meets naming and packaging guidelines - OK
- Spec file matches base package name. - OK
- Spec has consistent macro usage. - ***

The 'v' in the version seems extraneous. In fact, there seem to be identical
tarballs on the upstream download site with and without the 'v'.

- Meets Packaging Guidelines. - OK

- License - BSD ***

The code doesn't specify a license, so it's assumed from the included LICENSE
file. Would be nice if the code made it explicit.

Given the keys/signing involved in the build process, it would have been highly
entertaining if this package was GPL3. Alas, it is not.

- License field in spec matches - OK
- License file included in package - OK
- Spec in American English - OK
- Spec is legible  - OK ***

'An' in the summary is probably superfluous.

- Sources match upstream md5sum: - OK

- Package needs ExcludeArch - N/A
- BuildRequires correct - OK
- Package has %defattr and permissions on files is good. - OK
- Package has a correct %clean section. - OK
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) - OK
- Package is code or permissible content. - OK
- Doc subpackage needed/used. - ***

There are READMEs for both the daemon and the wireless regdb that should
probably be in %doc.

- Packages %doc files don't affect runtime. - OK

- Headers/static libs in -devel subpackage. - N/A

- Package compiles and builds on at least one arch. - OK (tested rawhide
x86_64, F10 x86)
- 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. - ***

Should own /usr/lib/crda

- No rpmlint output. - ***

crda.i386: W: non-conffile-in-etc /etc/udev/rules.d/regulatory.rules

See below about udev rules.

crda.i386: E: only-non-binary-in-usr-lib

Can be ignored.

- final provides and requires are sane: - ***

Arguably, should require udev.

SHOULD Items:

- Should build in mock. - OK (tested i386)
- Should build on all supported archs - didn't test
- Should function as described. - didn't test, don't have appropriate drivers
- Should have sane scriptlets. - N/A
- Should have dist tag - OK
- Should package latest version - OK

Random notes not covered above:
- We end up building two different upstreams into one package here. It could be
done with the wireless db built as a separate package, that includes its pubkey
in /etc/pki for the daemon to build later. If the daemon and the regulatory
information are going to be updated asynchronously, that might be worthwhile

- system udev rules should go in /lib/udev/rules.d, and usually are named
XX-regulatory.rules, where XX is some relative numeric priority

- the regulatory db is on /usr ... will it be needed before /usr is mounted?

- the usage flow seems to be 'user sets a domain -> netlink message to kernel
-> uevent -> udev helper to daemon -> reads database -> new netlink message to
kernel -> implements restrictions in driver. Seems overly convoluted, but..
meh.

- we remove the upstream key, and sign the regulatory db with our own key,
generated at build time (and then thrown away). As I understand it, this means
users won't be able to drop in new upstream releases of the regulatory db. Is
this intentional?

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review

Reply via email to