Bug 575332
Summary: | Review Request: qbzr - Bazaar plugin for Qt interface to most Bazaar operations | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Toshio Ernie Kuratomi <a.badger> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, notting, rdieter, terje.rosten |
Target Milestone: | --- | Flags: | rdieter:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | qbzr-0.18.4-1.fc13 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-04-13 12:42:29 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
Toshio Ernie Kuratomi
2010-03-20 05:26:54 UTC
Seems good, some comments: - add -k to dos2unix and move to %prep - more explict file listing : %{python_sitearch}/* - add comment about this issue? if test "%{python_sitelib}" != "%{python_sitearch}" ; then - I fail to see how using %{name} macro in source url help anything. Changes made: Spec URL: http://toshio.fedorapeople.org/packages/qbzr.spec SRPM URL: http://toshio.fedorapeople.org/packages/qbzr-0.18.3-2.fc12.src.rpm I few initial questions, 1. This %if ! (0%{?fedora} > 12 || 0%{?rhel} > 5) construct shouldn't be necessary, is it? the %{!?python_sitelib: ... conditionals should be sufficient, no? (not that it hurts anything, just curious) 2. why is this an arch-dependent noarch pkg exactly? 3. why do the files need to be in python_sitearch vs python_sitelib? (or perhaps the answer(s) to 2,3 are interrelated) f12 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2065278 $ rpmlint *.rpm x86_64/*.rpm qbzr.src: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging qbzr.src: W: spelling-error %description -l en_US frontends -> front ends, front-ends, Frontenac qbzr.src: W: spelling-error %description -l en_US bzr -> bar, brr, bur qbzr.src: W: spelling-error %description -l en_US dialogs -> dialog, dialog s, dialings qbzr.x86_64: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging qbzr.x86_64: W: spelling-error %description -l en_US frontends -> front ends, front-ends, Frontenac qbzr.x86_64: W: spelling-error %description -l en_US bzr -> bar, brr, bur qbzr.x86_64: W: spelling-error %description -l en_US dialogs -> dialog, dialog s, dialings qbzr.x86_64: E: no-binary qbzr.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages/bzrlib/plugins/qbzr/lib/uifactory.py 0644 /usr/bin/env These look mostly harmless, though I'll leave it to you if/how you want deal with uifactory.py (In reply to comment #3) > I few initial questions, > > 1. This > %if ! (0%{?fedora} > 12 || 0%{?rhel} > 5) > construct shouldn't be necessary, is it? the > %{!?python_sitelib: ... > conditionals should be sufficient, no? > (not that it hurts anything, just curious) > Correct. I put it in to document for myself (or future packagers) when it will no longer be necessary but the "!?" portion of the python_sitelib definition should be sufficient. > 2. why is this an arch-dependent noarch pkg exactly? > > 3. why do the files need to be in python_sitearch vs python_sitelib? > > (or perhaps the answer(s) to 2,3 are interrelated) Yeah -- bzr itself has a C extension and so it installs into %{python_sitearch}/bzrlib. This is a plugin to bzr and bzr only searches for plugins in its plugin directory: %{python_sitearch}/bzrlib/plugins So it has to be built with knowledge of %{_libdir} on different platforms, hence it can't be noarch. > qbzr.x86_64: E: non-executable-script > /usr/lib64/python2.6/site-packages/bzrlib/plugins/qbzr/lib/uifactory.py 0644 > /usr/bin/env Looked at this one -- it has a shebang line because it can be executed to test its functionality. I tend to leave the shebang line in but not make the file executable when that's the case (as upstream won't take a patch to remove the shebang but we don't really have a need to make it executable when we install it from a system package.) upstream source verified, md5sum *.gz 881e343f4808e8c8f0fbdbf4cce35d43 qbzr-0.18.3.tar.gz naming ok macros ok licensing ok APPROVED please document in the specfile about the arch-dependent noarch business. New Package CVS Request ======================= Package Name: qbzr Short Description: Bazaar plugin for Qt interface to most Bazaar operations Owners: toshio Branches: F-12 F-13 EL-5 InitialCC: Spec file updated locally with a better comment. I'll update once imported. CVS done (by process-cvs-requests.py). qbzr-0.18.4-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/qbzr-0.18.4-1.fc13 qbzr-0.18.4-1.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/qbzr-0.18.4-1.fc12 qbzr-0.18.4-1.fc12 has been pushed to the Fedora 12 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 qbzr'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/qbzr-0.18.4-1.fc12 qbzr-0.18.4-1.fc13 has been pushed to the Fedora 13 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 qbzr'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/qbzr-0.18.4-1.fc13 qbzr-0.18.4-1.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. qbzr-0.18.4-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. |