Bug 596746 - Review Request: bzr-explorer - GUI application for using Bazaar
Review Request: bzr-explorer - GUI application for using Bazaar
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Terje Røsten
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-27 08:43 EDT by Julian Aloofi
Modified: 2010-06-28 14:12 EDT (History)
4 users (show)

See Also:
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 17:48:33 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
terjeros: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Julian Aloofi 2010-05-27 08:43:52 EDT
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
Description: Bazaar Explorer is a desktop application for using the
Bazaar Version Control System. It provides a high level
interface to all commonly used features.

Functionality of this package can be extended by
installing bzr-gtk.
Comment 1 Julian Aloofi 2010-05-27 08:45:12 EDT
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.
Comment 2 Terje Røsten 2010-05-27 11:09:06 EDT
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
Comment 3 Terje Røsten 2010-05-27 17:33:28 EDT
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
Comment 4 Julian Aloofi 2010-05-28 07:04:34 EDT
(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
Comment 5 Terje Røsten 2010-05-29 05:52:12 EDT
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?
Comment 6 Julian Aloofi 2010-05-29 06:31:11 EDT
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.
Comment 7 Alexander Belchenko 2010-05-29 14:39:15 EDT
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.
Comment 8 Julian Aloofi 2010-05-29 15:33:54 EDT
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.
Comment 9 Alexander Belchenko 2010-05-29 16:07:31 EDT
Actually there is present bzr icon in the Bazaar Explorer sources: see extras/bzr-48.bmp. Although it's not PNG :-/
Comment 10 Terje Røsten 2010-05-29 16:56:28 EDT
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
Comment 11 Julian Aloofi 2010-05-29 17:33:45 EDT
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).
Comment 12 Alexander Belchenko 2010-05-30 03:24:25 EDT
IIRC there is no bzr.png in the main bzr tarball, only bzr.ico (Windows-style icon).
Comment 13 Julian Aloofi 2010-05-30 07:22:50 EDT
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.
Comment 14 Toshio Ernie Kuratomi 2010-05-30 14:39:55 EDT
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
Comment 15 Julian Aloofi 2010-05-31 07:30:23 EDT
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.
Comment 16 Fedora Update System 2010-05-31 15:15:05 EDT
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
Comment 17 Fedora Update System 2010-05-31 15:15:14 EDT
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
Comment 18 Fedora Update System 2010-05-31 15:15:20 EDT
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
Comment 19 Toshio Ernie Kuratomi 2010-05-31 15:22:13 EDT
This update places a /usr/share/pixmaps/bzr.png icon.

For reference:  upstream bug about icons: https://bugs.launchpad.net/bzr/+bug/245602
Comment 20 Julian Aloofi 2010-06-01 10:03:01 EDT
Great, I'll post a new SRPM and spec using the icon and with a new description and summary as soon as possible.
Comment 21 Julian Aloofi 2010-06-01 14:18:43 EDT
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)
Comment 22 Terje Røsten 2010-06-02 14:15:36 EDT
Thanks, review in progress.
Comment 23 Terje Røsten 2010-06-02 14:23:19 EDT
I noted bzr-explorer 1.0.2 was released, could you update the package to 1.0.2 before I continue?
Comment 24 Julian Aloofi 2010-06-03 08:36:52 EDT
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.
Comment 25 Terje Røsten 2010-06-03 09:04:07 EDT
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.
Comment 26 Julian Aloofi 2010-06-03 09:37:32 EDT
New Package CVS Request
=======================
Package Name: bzr-explorer
Short Description: Graphical application for using Bazaar
Owners: julian
Branches: F-13
InitialCC:
Comment 27 Julian Aloofi 2010-06-03 10:43:24 EDT
Thanks for reviewing, by the way! :)
Comment 28 Kevin Fenzi 2010-06-03 16:30:09 EDT
CVS done (by process-cvs-requests.py).
Comment 29 Terje Røsten 2010-06-09 13:14:09 EDT
Seems like you have imported and built, please close the ticket if no more work is needed.
Comment 30 Julian Aloofi 2010-06-09 17:48:33 EDT
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.
Comment 31 Fedora Update System 2010-06-18 04:20:59 EDT
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
Comment 32 Fedora Update System 2010-06-21 09:07:20 EDT
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.
Comment 33 Fedora Update System 2010-06-21 09:07:52 EDT
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.
Comment 34 Fedora Update System 2010-06-21 21:11:29 EDT
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.
Comment 35 Fedora Update System 2010-06-28 13:17:11 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.