Bug 475132
Summary: | Review Request: usbmon - Front-end for in-kernel usbmon | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Pete Zaitcev <zaitcev> |
Component: | Package Review | Assignee: | Kevin Fenzi <kevin> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mail, notting |
Target Milestone: | --- | Flags: | kevin:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2008-12-26 20:15:16 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: |
Description
Pete Zaitcev
2008-12-08 03:30:18 UTC
Just some quick comments on your spec file - 'Version: 4.05', the latest version is 4.06 - 'Group: File tools' is not a valid group. -> 'less /usr/share/doc/rpm-*/GROUPS' for a list with valid group entries - 'Requires: perl...' is missing, take a look at https://fedoraproject.org/wiki/Packaging/Perl#Versioned_MODULE_COMPAT__Requires - '%defattr(-,root,root)' is normally '%defattr(-,root,root,-)' - Your Changelog entry is missing the Release. See https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs The comments above apparently belong to some different package. Hmmm, yes the comments are for clamtk. Sorry. One for this package: 'Release: 1' should be 'Release: 1%{dist}' (In reply to comment #3) > One for this package: 'Release: 1' should be 'Release: 1%{dist}' Fixed N.B. This changes the SRPM URL: http://people.redhat.com/zaitcev/tmp/usbmon-5.2-1.fc11.src.rpm I'd be happy to review this and sponsor you... look for a full review in a bit. OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPLv2) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: cbba81a5b47b811dafd897cd7bd72dee usbmon-5.2.tar.gz cbba81a5b47b811dafd897cd7bd72dee usbmon-5.2.tar.gz.orig See below - Package needs ExcludeArch OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. OK - 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. OK - No rpmlint output. OK - final provides and requires are sane SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version Issues: A few general comments, unrelated to the packaging and thus moot for the review, but I thought I would mention them: - You have your upstream url as your people.redhat.com page. Perhaps it would be good to use a fedorahosted.org site for this? That way you get bug tracking/mailing lists/vcs repo, etc. See: https://fedorahosted.org/web/new - You might add a note about the license version to the .c file. No big deal since it's mentioned other places and is clearly your intent, but just to be paranoid. ;) Now, on to issues about the package: 1. I have no idea on the ExcludeArch. Does s390 have usb? In any case it's not a blocker here as Fedora doesn't have s390 as a primary arch. You might ask the s390 list? I don't see any other blockers here, this is a very simple package, and is APPROVED. Go ahead and continue the process from http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account If you have any questions don't hesitate to contact me via bugzilla, email, or on irc (nirik on freenode). (In reply to comment #6) > - You have your upstream url as your people.redhat.com page. > Perhaps it would be good to use a fedorahosted.org site for this? > That way you get bug tracking/mailing lists/vcs repo, etc. > See: https://fedorahosted.org/web/new I hoped to get by, because thus far usbmon only received 3 patches from external contributors. It's a very simple, even trivial tool. But I'll keep this in mind, especially if they come out once they see it packaged. > - You might add a note about the license version to the .c file. > No big deal since it's mentioned other places and is clearly your intent, > but just to be paranoid. ;) How about I do it in 5.3, so as not to invalidate the approval of 5.2? >I hoped to get by, because thus far usbmon only received 3 patches >from external contributors. It's a very simple, even trivial tool. >But I'll keep this in mind, especially if they come out once they >see it packaged. Yeah, may be more overhead than needed now, but something to consider. > How about I do it in 5.3, so as not to invalidate the approval of 5.2? If you want to post a updated spec/src.rpm, I would be happy to look it over. Or you can just import it with that change... thats fine as well. New Package CVS Request ======================= Package Name: usbmon Short Description: The front-end for usbmon Owners: zaitcev Branches: F-9 F-10 InitialCC: zaitcev cvs done. "yum install usbmon" works in Rawhide (10.90), closing as NEXTRELEASE. |