Bug 253649 - Review Request: vinagre - VNC client for the GNOME desktop
Review Request: vinagre - VNC client for the GNOME desktop
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ray Strode [halfline]
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-20 18:57 EDT by Bastien Nocera
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version: 0.2-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-22 12:07:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rstrode: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bastien Nocera 2007-08-20 18:57:17 EDT
Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-1.src.rpm
Description: Vinagre is a VNC client for the GNOME desktop environment.

It uses gtk-vnc. Upstream is at:
http://www.gnome.org/~jwendell/vinagre/

(Note that 0.2 doesn't appear there, because I asked Jonh to do a release for me, so I could get to upload this package before feature freeze).
Comment 1 Ray Strode [halfline] 2007-08-21 11:14:23 EDT
When building in mock I get:


checking for XML::Parser... configure: error: XML::Parser perl module is
required for intltool

I've seen that problem before although I don't remember the details.  Is it that
the packaged intltool requires XML::Parser?  I'm guessing either BuildRequires
the parser or BuildRequires intltool and re-intltoolize will fix it.

Overall looks very good.  Minor nits that don't matter:

- rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/vinagre/  has an extra / in it (between
ROOT and %{_datadir}
- what docs are in the above anyway?  omf files? Are they worth shipping?
- vendor isn't required anymore for desktop-file-install, just ditch it.
- There are a handful or so Application categories, normally when tossing the
Application category, you pick one of them to replace it with (Maybe use Network?)
- you have %{_datadir}/%{name}/* in the file list but the package should
probably own the %{_datadir}/%{name} dir too.
Comment 2 Bastien Nocera 2007-08-21 11:23:21 EDT
(In reply to comment #1)
> When building in mock I get:
> 
> 
> checking for XML::Parser... configure: error: XML::Parser perl module is
> required for intltool
> 
> I've seen that problem before although I don't remember the details.  Is it that
> the packaged intltool requires XML::Parser?  I'm guessing either BuildRequires
> the parser or BuildRequires intltool and re-intltoolize will fix it.

added perl-XML-Parser as a BR

> Overall looks very good.  Minor nits that don't matter:
> 
> - rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/vinagre/  has an extra / in it (between
> ROOT and %{_datadir}

Fixed.

> - what docs are in the above anyway?  omf files? Are they worth shipping?

No, they're the usual AUTHORS, README, etc., but the maintainer thought it was
good to install them by himself. I'm removing those, and installing in the usual
versioned docs dir in %files

> - vendor isn't required anymore for desktop-file-install, just ditch it.

Done.

> - There are a handful or so Application categories, normally when tossing the
> Application category, you pick one of them to replace it with (Maybe use Network?)

It's already in Network, so appears in the same menu as TSClient

> - you have %{_datadir}/%{name}/* in the file list but the package should
> probably own the %{_datadir}/%{name} dir too.

Fixed, we want the whole dir.

Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-2.src.rpm
Comment 3 Ray Strode [halfline] 2007-08-21 11:38:21 EDT
change that added BuildRequires to

BuildRequires: perl(XML::Parser)

and add one for libglade2-devel
Comment 4 Bastien Nocera 2007-08-21 11:42:24 EDT
My mistake, fixed here:
Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-3.src.rpm
Comment 5 Ray Strode [halfline] 2007-08-21 12:02:25 EDT
looks like avahi-ui-devel is missing a Requires: avahi-glib-devel which is
breaking mock too, but that's not a problem with this package.
Comment 6 Bastien Nocera 2007-08-21 12:10:36 EDT
Filed as bug 253734

Worked around in:
Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-4.src.rpm
Comment 7 Bastien Nocera 2007-08-21 12:12:49 EDT
More missing deps, this time it's gettext and co.

Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-5.src.rpm
Comment 8 Ray Strode [halfline] 2007-08-21 12:30:10 EDT
Also need BuildRequires: desktop-file-utils
Comment 9 Bastien Nocera 2007-08-21 12:35:02 EDT
And missing desktop-file-utils:
Spec URL: http://people.redhat.com/bnocera/vinagre/vinagre.spec
SRPM URL: http://people.redhat.com/bnocera/vinagre/vinagre-0.2-6.src.rpm
Comment 10 Ray Strode [halfline] 2007-08-21 12:47:13 EDT
looks good.
Comment 11 Bastien Nocera 2007-08-21 13:15:48 EDT
Thanks Ray!

New Package CVS Request
=======================
Package Name: vinagre
Short Description: VNC client for the GNOME desktop
Owners: hadess
Branches: devel
InitialCC: bnocera@redhat.com
Commits by cvsextras:yes
Comment 12 Kevin Fenzi 2007-08-21 19:33:28 EDT
cvs done.
Comment 13 Bastien Nocera 2007-08-22 12:07:36 EDT
Uploaded to rawhide.

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