Bug 478432
Summary: | Review Request: dwscan - Displays access point information | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Fabian Affolter <mail> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dominik, fedora-package-review, notting, rc040203 |
Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 0.2-3.fc10 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-04-09 16:16:17 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: | 478300 | ||
Bug Blocks: |
Description
Fabian Affolter
2008-12-29 23:01:16 UTC
Some remarks: - Your patch is superfluous. Overriding at make-time: make install bindir=%{_bindir} has the same effect without hacking the code. - The Makefile doesn't honor INSTALL => make INSTALL="install -p" is non-functional Furthermore: - I am having strong doubts if installing this tool to %{_bindir} is a good idea. Thanks Ralf for your inputs. (In reply to comment #1) > - Your patch is superfluous. > Overriding at make-time: > make install bindir=%{_bindir} > has the same effect without hacking the code. It's much smarter. Changed > - The Makefile doesn't honor INSTALL > => make INSTALL="install -p" > is non-functional fixed > Furthermore: > - I am having strong doubts if installing this tool to %{_bindir} is a good > idea. What location do you have in mind? Here are the updated files. Spec URL: http://fab.fedorapeople.org/packages/SRPMS/dwscan.spec SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/dwscan-0.2-2.fc9.src.rpm I'm guessing that Ralf's concern was that the program needs privilege to run and so should be installed in /usr/sbin. Of course, the current default path kind of eliminates the difference, but upstream seems to agree. Do you have any thoughts on this? The software seems confused about its license. The actual program clearly indicates LGPLv2 (only) but COPYING and the included spec say GPLv2. The software itself overrides, of course, but then I don't see the value in packaging the useless COPYING file. Could you ping upstream and see if you can get them to clarify? Are you sure this really needs python to build? It doesn't seem to me like it does. * source files match upstream. sha256sum: 84d2161d1b9714d4517cef811a35fe53fdddfd5b22a2693811ccae02ed1ef779 dwscan-0.2.tar.bz2 * package meets naming and versioning guidelines. specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * build root is OK. X license field does not match the actual license. * license is open source-compatible. * latest version is being packaged. X BuildRequires * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * rpmlint is silent. * final provides and requires are sane: dwscan = 0.2-2.fc11 = /usr/bin/python python-wifi * owns the directory it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. The package review process needs reviewers! If you haven't done any package reviews recently, please consider doing one. (In reply to comment #3) > I'm guessing that Ralf's concern was that the program needs privilege to run > and so should be installed in /usr/sbin. Right. > Of course, the current default path > kind of eliminates the difference, Well, I guess my opinion on this is known: I consider it to be a severe mistake. > but upstream seems to agree. I am not familiar with this particular package's upstream, however many upstreams simply tend to not taking "system integration" consideration into account. My evidence that upstream seems to agree is simply that the makefile installs the binary into "sbindir". Only the passing of sbindir="%{_bindir} in the make call gets the executable into /usr/bin. (In reply to comment #3) > I'm guessing that Ralf's concern was that the program needs privilege to run > and so should be installed in /usr/sbin. Of course, the current default path > kind of eliminates the difference, but upstream seems to agree. Do you have > any thoughts on this? I will leave the path untouched. > The software seems confused about its license. The actual program clearly > indicates LGPLv2 (only) but COPYING and the included spec say GPLv2. The > software itself overrides, of course, but then I don't see the value in > packaging the useless COPYING file. Could you ping upstream and see if you can > get them to clarify? COPYING removed and LGPLv2 added as license tag for now till upstream release a new source tarball. > X license field does not match the actual license. LGPLv2 for now. > X BuildRequires fixed Updated files: Spec URL: http://fab.fedorapeople.org/packages/SRPMS/dwscan.spec SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/dwscan-0.2-3.fc10.src.rpm Could you explain in the spec why you wish to override upstream's placement of the binary in /usr/sbin, then? I'm not sure there's really any reasonable justification for it, but if you have one then I'll be happy to approve the package. (In reply to comment #7) > Could you explain in the spec why you wish to override upstream's placement of > the binary in /usr/sbin, then? I'm not sure there's really any reasonable > justification for it, but if you have one then I'll be happy to approve the > package. I will not override the placement of the binary. I reversed this in Release 3 of the package. Sorry, I must have misunderstood your comment. Everything looks fine to me now. APPROVED Thanks again for your review. New Package CVS Request ======================= Package Name: dwscan Short Description: Displays access point information Owners: fab Branches: F-9 F-10 InitialCC: cvs done. shtool-2.0.8-2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/shtool-2.0.8-2.fc9 shtool-2.0.8-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/shtool-2.0.8-2.fc10 dwscan-0.2-3.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/dwscan-0.2-3.fc9 dwscan-0.2-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/dwscan-0.2-3.fc10 dwscan-0.2-3.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update dwscan'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3162 dwscan-0.2-3.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing-newkey update dwscan'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-3119 dwscan-0.2-3.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. dwscan-0.2-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. |