[Bug 177583] Review Request: zaptel-kmod

[bugzilla at redhat.com]

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: zaptel-kmod


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


kevin at tummy.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|gdk at redhat.com              |kevin at tummy.com
OtherBugsDependingO|163776                      |163778
              nThis|                            |




------- Additional Comments From kevin at tummy.com  2006-06-16 21:18 EST -------
ok, finally found some time to reboot my main asterisk box to the
latest fc5 kernel so I could try this out. Everythings working fine
with your:

asterisk-zaptel-1.2.9.1-1.fc5
asterisk-zaptel-1.2.9.1-1.fc5
asterisk-sounds-default-1.2.9.1-1.fc5
asterisk-1.2.9.1-1.fc5
zaptel-1.2.6-3.fc5
kmod-zaptel-smp-1.2.6-6.2.6.16_1.2133_FC5

So, time to start in on some reviews. :)

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
N/A - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
c6058b74f43ae12a29e486cf1e919562  zaptel-1.2.6.tar.gz
c6058b74f43ae12a29e486cf1e919562  zaptel-1.2.6.tar.gz.1
OK - Package compiles and builds on at least one arch.
N/A - Package needs ExcludeArch
OK - BuildRequires correct
N/A - Spec handles locales/find_lang
N/A - Spec has needed ldconfig in post and postun
N/A - Package is relocatable and has a reason to be.
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
N/A - -doc subpackage needed/used.
N/A - Packages %doc files don't affect runtime.
N/A - Headers/static libs in -devel subpackage.
N/A - .pc files in -devel subpackage.
N/A - .so files in -devel subpackage.
N/A - -devel package Requires: %{name} = %{version}-%{release}
N/A - .la files are removed.
N/A - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output.

Issues:

1. Still need the "A publishable explanation from the author(s) why
the module is not merged with the mainline kernel yet and when it's
planed to get merged. You of course can ask the author to explain
it directly in the bug report." and approval from
FESCo at the next meeting.

2. Fair pile of rpmlint output, most of which can be ignored I think:

This would need to be fixed in kmodtool:
W: kmod-zaptel summary-not-capitalized zaptel kernel module(s)

W: kmod-zaptel unstripped-binary-or-object
/lib/modules/2.6.16-1.2133_FC5/extra/zaptel/wctdm.ko
(repeats for each .ko in each subpackage.)

I think this one is due to the name of the package vs the postin...
the scriptlet has the right kernel name, the package has a extra _
in place of a -

E: kmod-zaptel postin-with-wrong-depmod
/lib/modules/2.6.16-1.2133_FC5/extra/zaptel/zaptel.ko
(repeats for each .ko in each subpackage.)

Can be ignored:

W: kmod-zaptel no-documentation

If you are not applying these, perhaps they should be dropped:

W: zaptel-kmod patch-not-applied Patch0: zaptel-config.patch
W: zaptel-kmod patch-not-applied Patch2: zaptel-optflags.patch

3. "Reviewers of kernel modules should diff the proposed kernel module
packages against the template. Only the names and the way the modules
 itself are build should differ.  There shouldn't be other differences
 without a good reason."

I can't seem to get the current template from the wiki. The link seems to
be:

http://fedoraproject.org/wiki/Packaging/KernelModules?action=AttachFile&do=get&target=kmod-template.spec

which doesn't work.

4. Finally: (Although we aren't at approval yet)

"Everyone can review such a package, but after is was set to
APPROVED by the reviewer a Fedora Extras Sponsor or someone
experienced with kernel modules has to take a quick look at
the package and post an additional approved notice before
it is allowed to import the package into CVS."

It would be great if one of the experenced kernel module folks
could look over this once we reach approval.



-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.