Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 596746
Summary: | Review Request: bzr-explorer - GUI application for using Bazaar | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Julian Aloofi <julian.fedora> |
Component: | Package Review | Assignee: | Terje Røsten <terjeros> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | a.badger, fedora-package-review, notting, terjeros |
Target Milestone: | --- | Flags: | terjeros:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | bzr-explorer-1.0.2-1.fc13 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-06-09 21:48:33 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
Julian Aloofi
2010-05-27 12:43:52 UTC
rpmlint reports a spelling mistake regarding 'gtk' [makerpm@Julians-Notebook rpmbuild]$ rpmlint SRPMS/bzr-explorer-1.0.1-1.fc13.src.rpm SPECS/bzr-explorer.spec RPMS/noarch/bzr-explorer-1.0.1-1.fc13.noarch.rpm bzr-explorer.src: W: spelling-error %description -l en_US gtk -> gt, gt k, GTE bzr-explorer.noarch: W: spelling-error %description -l en_US gtk -> gt, gt k, GTE 2 packages and 1 specfiles checked; 0 errors, 2 warnings. Thanks for packaging this, I will have a closer look later. btw :bzr-gtk is orphaned in rawhide and seeking a maintainer: https://admin.fedoraproject.org/pkgdb/acls/name/bzr-gtk Some more comments: - use version macro in source url - summary is a bit short? - don't builds in F12 (missing defs) okay? - use globbing for egg-info to simplify upgrade? pedantic - remove some empty lines - some places you use %{name}, some you don't. For later reference, koji builds it fine: http://koji.fedoraproject.org/koji/taskinfo?taskID=2214002 (In reply to comment #3) > - use version macro in source url > - use globbing for egg-info to simplify upgrade? Right, where were I thinking? :D > - don't builds in F12 (missing defs) okay? bzr-explorer requires bzr 2.1, which isn't in Fedora 12 anyway as far as I can see ( https://admin.fedoraproject.org/updates/bzr ). I haven't tried building it on Fedora 12, but I'm trusting upstream on this one. > - summary is a bit short? It gets found when searching for bzr and GUI and sums it up nicely (well, at least in my opinion). But Debian's description is "GUI application for using bazaar", and so is the .desktop's file, so I guess it is a good idea to change it. > - some places you use %{name}, some you don't. Yeah, I admit I skipped the "find name and version usage and replace" step, will adjust that now. > > For later reference, koji builds it fine: > > http://koji.fedoraproject.org/koji/taskinfo?taskID=2214002 I did a build against dist-f13 as well (as mock was causing some errors) http://koji.fedoraproject.org/koji/taskinfo?taskID=2212591 So here are the new spec and SRPM: Spec URL: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer.spec SRPM URL: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer-1.0.1-1.fc13.src.rpm Thanks. I don't like abbreviation as GUI in summary, but it's up to you. Desktop file don't have icon shipped, could you take e.g. https://launchpadlibrarian.net/28245614/bazaar-explorer-64.png and put into /usr/share/pixmaps/ and adjust desktop file to use it? The icon looks good, but I'll rather ask upstream first which icon they'd like to see being used. Regarding the description, it's in line with Debian and Ubuntu (and the desktop file), but I also think "Graphical" may sound a bit better. I'll change this as well. Hi, I'm one the of maintainers of Bazaar Explorer. I'm not quite sure about "extended" in the following line: "Functionality of this package can be extended by installing bzr-gtk." Actually Bazaar Explorer is front-end application which can use either QBzr or bzr-gtk for executing specific actions. In the same time QBzr is strongly required to run Bazaar Explorer, because Explorer using some code from QBzr. In the same time QBzr dialogs used in Explorer (I think by default). So maybe it will be better to say that user can use bzr-gtk if he/she want different action dialogs. Feel free to ask if you need additional clarifications. Okay, that sounds a bit vague, I agree. I'll change it to "If you want to use different action dialogs, you can additionally install bzr-gtk." Regarding packaging now, I don't know that it's wise to provide /usr/share/pixmaps/bzr.png in this package. I think it would be better to move the icon to the main bzr package. I'll send a mail to the maintainer now. Actually there is present bzr icon in the Bazaar Explorer sources: see extras/bzr-48.bmp. Although it's not PNG :-/ Easy to fix, patch to convert to png and ship bzr.png: +++ bzr-explorer.spec 2010-05-29 22:54:54.000000000 +0200 @@ -21,6 +21,8 @@ BuildRequires: python-distutils-extra BuildRequires: gettext BuildRequires: desktop-file-utils +# For convert +BuildRequires: ImageMagick Requires: qbzr >= 0.18 Requires: bzr >= 2.1 @@ -40,6 +42,7 @@ %build %{__python} setup.py build +convert extras/bzr-48.bmp bzr.png %install @@ -56,6 +59,7 @@ #put the locale into the right dir, see issue mentioned at Patch0 mkdir -p %{buildroot}%{_datadir} mv %{buildroot}%{python_sitelib}/bzrlib/plugins/%{explorer}/locale %{buildroot}%{_datadir} +install -D -p -m 0644 bzr.png %{buildroot}%{_datadir}/pixmaps/bzr.png %find_lang %{explorer} @@ -70,6 +74,7 @@ %{python_sitelib}/bzrlib/plugins/%{explorer}/ %{python_sitelib}/*.egg-info %{_datadir}/applications/%{name}.desktop +%{_datadir}/pixmaps/bzr.png %doc README.txt COPYING.txt NEWS doc Yeah, the thing is, I don't really want to provide /usr/share/pixmaps/bzr.png, as it's a pretty common name, and bzr-gtk already provides the 64x64 png version of the icon. I sent a mail to the bzr and bzr-gtk maintainer and asked him whether it would be okay with him to move the bzr.png to the main bzr package, as it's kind of a common resource. I'll wait for his answer, but there's always the option to provide a bzr.png in bzr-explorer, and accept the (little) duplication, as bzr-gtk also has such an icon (although under another name, bzr-64.png if I remember correctly). IIRC there is no bzr.png in the main bzr tarball, only bzr.ico (Windows-style icon). But the maintainer of our main bzr package could put an icon in that, so we wouldn't have the icon in both bzr-gtk and bzr-explorer. Then they could both use the icon from the main bzr package as they depend on it anyway. Yep, I'm willing to move things around. So -- do we want: /usr/share/pixmaps/bzr-64.png for everything? Or: /usr/share/pixmaps/bzr.png I don't mind personally, but I don't see many other icons with a resolution in the file name, so I'd say bzr.png is probably best. bzr-2.0.5-2.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/bzr-2.0.5-2.fc12 bzr-2.1.1-2.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/bzr-2.1.1-2.fc13 bzr-2.1.1-4.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/bzr-2.1.1-4.el5 This update places a /usr/share/pixmaps/bzr.png icon. For reference: upstream bug about icons: https://bugs.launchpad.net/bzr/+bug/245602 Great, I'll post a new SRPM and spec using the icon and with a new description and summary as soon as possible. New Spec: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer.spec New SRPM: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer-1.0.1-3.fc13.src.rpm Note: To see the icon in the desktop file, you'll need bzr >= 2.1.1-2 (in updates-testing at this point) Thanks, review in progress. I noted bzr-explorer 1.0.2 was released, could you update the package to 1.0.2 before I continue? Sure thing! New Spec: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer.spec New SRPM: http://julian.fedorapeople.org/bzr-explorer/bzr-explorer-1.0.2-1.fc13.src.rpm Note: Still requires bzr from updates-testing. Thanks, formal review: ok rpmlint, only harmless spelling warnings ok naming of package and spec ok spec file include doc/ROADMAP.txt and doc/overview.txt? ok license approved and tag ok. GPL2+, all files seems to have license header ok license in %doc ok correct language ok sha1sum on sources and ok url sha1sum bzr-explorer-1.0.2.tar.gz* ad57c9cbcde2004af76ed29b520ddd075c2d15fe bzr-explorer-1.0.2.tar.gz ad57c9cbcde2004af76ed29b520ddd075c2d15fe bzr-explorer-1.0.2.tar.gz.orig ok koji build with correct buildreq http://koji.fedoraproject.org/koji/taskinfo?taskID=2227301 ok excludearch ok locale files - ldconfig ok no bundling ok owns, dirs and perms and only once ok macros ok code or content - large docs ok %doc not affect the runtime - headers|static in devel|static - .so in devel - devel dep on base - no .la|.a file ok gui with desktop file ok own just not owned ok utf-8 file names Thanks for working with upstream and fixing the issues that was discovered. The package bzr-explorer is APPROVED. New Package CVS Request ======================= Package Name: bzr-explorer Short Description: Graphical application for using Bazaar Owners: julian Branches: F-13 InitialCC: Thanks for reviewing, by the way! :) CVS done (by process-cvs-requests.py). Seems like you have imported and built, please close the ticket if no more work is needed. I planned to close it with the push of it to the F13 stable updates, but the new bzr is hanging around in updates-testing a while. I'll just close this now. bzr-explorer-1.0.2-1.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/bzr-explorer-1.0.2-1.fc13 bzr-2.1.1-2.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. bzr-2.0.5-2.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. bzr-2.1.1-4.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. bzr-explorer-1.0.2-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. |