Bug 450539 - Review Request: service-discovery-applet - Service discovery applet based on Avahi for the Gnome panel
Review Request: service-discovery-applet - Service discovery applet based on ...
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
NotReady
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-09 09:37 EDT by Tim Niemueller
Modified: 2011-04-21 18:14 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-04-21 18:14:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Replacement VNC plugin for Service Discovery Applet (1.72 KB, text/plain)
2009-01-12 09:37 EST, Noel J. Bergman
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
Launchpad 130334 None None None Never

  None (edit)
Description Tim Niemueller 2008-06-09 09:37:28 EDT
Spec URL: http://fedorapeople.org/~timn/misc/service-discovery-applet.spec
SRPM URL: http://fedorapeople.org/~timn/misc/service-discovery-applet-0.4.5-0.2.svn20080609.fc9.src.rpm
Description:
The service discovery applet lists all available services which are published
through Avahi/Rendezvous/Bonjour/ZeroConf and allows to run actions on that
service.

Website: has none, mentioned on http://avahi.org/wiki/AdministrativeAvahiApplication, SVN at http://svn.0pointer.de/viewvc/trunk/?root=service-discovery-applet
Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=653703

Subversion package contains more functionality than latest release (very old), thus svn-version is packaged.
Comment 1 Christoph Wickert 2008-06-29 11:07:27 EDT
Not an official review, just a few thoughts:

I suggest to name this package gnome-applet-service-discovery, see
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28General.29

Better name a svn snapshot after the revision and not the date, because then the
reviewer can simply check out the correrct version with
svn co -rXXXXX svn://svn.0pointer.de/service-discovery-applet/trunk
service-discovery-applet

I see you define both python_sitelib and python_sitearch. Please remove the
unneeded definition.

You are missing a requirement:
$ service-discovery-applet 
Traceback (most recent call last):
  File "/usr/bin/service-discovery-applet", line 51, in <module>
    error_msg(_("A required python module is missing!\n%s") % (e))
  File "/usr/bin/service-discovery-applet", line 29, in error_msg
    d = gtk.MessageDialog(parent=None, flags=gtk.DIALOG_MODAL,
NameError: global name 'gtk' is not defined

running service-discovery-config gives a messagebox saying:
"Ein benötigtes Python Modul fehlt!
No module named avahi"

Not sure if we have a python-avahi package
Comment 2 Tim Niemueller 2008-06-29 18:43:01 EDT
(In reply to comment #1)
> Not an official review, just a few thoughts:
> 
> I suggest to name this package gnome-applet-service-discovery, see
>
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28General.29

Hmm, this is different from upstream. My first search was for
service-discovery-applet so I wouldn't have found it with that name. Would yum
search provides lines to remedy this problem?

> Better name a svn snapshot after the revision and not the date, because then the
> reviewer can simply check out the correrct version with
> svn co -rXXXXX svn://svn.0pointer.de/service-discovery-applet/trunk
> service-discovery-applet

So you can with the date, just use svn co -r{DATE}... Having a date usually
makes it easier to get a feeling how up2date the package really is. I've used
this for other packages already. Just assume someone tried to do this on git or
cvs stuff...

> I see you define both python_sitelib and python_sitearch. Please remove the
> unneeded definition.

I usually keep it for easy spawning new packages by copying. I can't see in the
guidelines that I shouldn't.

> You are missing a requirement:
> $ service-discovery-applet 
> Traceback (most recent call last):
>   File "/usr/bin/service-discovery-applet", line 51, in <module>
>     error_msg(_("A required python module is missing!\n%s") % (e))
>   File "/usr/bin/service-discovery-applet", line 29, in error_msg
>     d = gtk.MessageDialog(parent=None, flags=gtk.DIALOG_MODAL,
> NameError: global name 'gtk' is not defined
> 
> running service-discovery-config gives a messagebox saying:
> "Ein benötigtes Python Modul fehlt!
> No module named avahi"
> 
> Not sure if we have a python-avahi package

Ah, needs to require avahi-tools, which contains the Python bindings for some
reason... Thanks for pointing this out!

I've uploaded a new SPEC (same URL as above) and new SRPM (at
http://fedorapeople.org/~timn/misc/service-discovery-applet-0.4.5-0.3.svn20080609.fc9.src.rpm).
Comment 3 Christoph Wickert 2008-06-30 19:51:22 EDT
(In reply to comment #2)
> My first search was for
> service-discovery-applet so I wouldn't have found it with that name. Would yum
> search provides lines to remedy this problem?

No, unfortunately not. 'yum install service-discovery-applet' would work but not
'yum search'. IMO this is something that should be fixed in yum. Nevertheless we
should stick to the naming guidelines I think.

> So you can with the date, just use svn co -r{DATE}... Having a date usually
> makes it easier to get a feeling how up2date the package really is. 

Ok, agreed as it's in line with the naming guidelines.

> I usually keep it for easy spawning new packages by copying. I can't see in the
> guidelines that I shouldn't.

It's written in the python spec template from rpmdev-newrpmspec, but in the end
it's your decision.

> Ah, needs to require avahi-tools, which contains the Python bindings for some
> reason... Thanks for pointing this out!

But I still don't see a Requires: on avahi-tools, only the BuildRequires that
has been there before. :(

> I've uploaded a new SPEC (same URL as above) and new SRPM (at
>
http://fedorapeople.org/~timn/misc/service-discovery-applet-0.4.5-0.3.svn20080609.fc9.src.rpm).

Ok, a few more comments on that one:

Why not change 
  # cd service-discovery-applet
  # svn export service-discovery-applet service-discovery-applet-0.4.5.svn`date
+%Y%m%d`
  # cd ..
to 
  # svn export . ../service-discovery-applet-0.4.5.svn`date +%Y%m%d`

  tar cvfz ...
needs to  be
  tar -cvfz ...

BuildRoot tag is wrong, according to the guidelines it should be
  "%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)"

BuildRequires "perl-XML-Parser" should be "perl(XML::Parser)" instead

Are you sure you need gettext-devel and not only gettext?

You have a lot of redundant build requirements that don't need to be specified
explicitly:
- libtool requires autoconf and automake, automake requires autoconf and perl
- GConf2-devel requires automake and glib2-devel
- intltool already requires gettext and perl(XML::Parser)
- perl(XML::Parser) of course requires perl
As long as you don't specify a version it is not necessary to list all these BRs.

Use "export GCONF_DISABLE_MAKEFILE_SCHEMA_INSTALL=1" to avoid error messages due
to missing permissions during %install. You are also missing the necessary
scriptlets to (un)install the gconf schemas, see
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf

Use "make %{?_smp_mflags}", although it won't make much difference.

Please fix %defattr to (-,root,root-). Only cosmetics, I know. ;)

%{_libdir}/bonobo/servers/GNOME_ServiceDiscoveryApplet.server
%{_libdir} will become /usr/lib64 on x86_64, so this package cannot be noarch.
Comment 4 Christoph Wickert 2008-06-30 20:15:22 EDT
(In reply to comment #3)
> Why not change 
>   # cd service-discovery-applet
>   # svn export service-discovery-applet service-discovery-applet-0.4.5.svn`date
> +%Y%m%d`
>   # cd ..
> to 
>   # svn export . ../service-discovery-applet-0.4.5.svn`date +%Y%m%d`

Oops, I 'fixed' the wrong line. ;( Should read:

Why not change 
  # cd service-discovery-applet
  # svn export . ../service-discovery-applet-0.4.5.svn`date +%Y%m%d`
  # cd ..
to 
  # svn export service-discovery-applet service-discovery-applet-0.4.5.svn`date
+%Y%m%d`
Comment 5 Christoph Wickert 2008-07-04 17:48:53 EDT
(In reply to comment #3)
> - intltool already requires gettext and perl(XML::Parser)

I have to correct myself: The F-8 intltool package doesn't require gettext (the
Requirements are completely messed up on F-8), so it's better to BuildRequire it
explicitly.

Tim, are there any problems with my suggestions/feedback? If so, please let me know.
Comment 6 Tim Niemueller 2008-07-04 18:10:54 EDT
I'll be out of the country for the next weeks, I'll be back at the end of July
and then I'll work on this again.
Comment 7 Fabian Affolter 2008-12-08 06:56:27 EST
An other small thing...I would suggest to switch the URL from 'http://0pointer.de/cgi-bin/viewcvs.cgi/?root=service-discovery-applet' to 'http://avahi.org', the upstream website.
Comment 8 Fabian Affolter 2009-01-12 05:08:09 EST
Is there any progress with this bug?
Comment 9 Noel J. Bergman 2009-01-12 09:34:18 EST
For what it is worth, I have a replacement for the VNC plugin.  Trying to get it into upstream, and will post here for your convenience.  It handles three of the common VNC viewers, so it is more distro and version independent, and provides an easy means to plugin any other VNC viewer that someone might want.
Comment 10 Noel J. Bergman 2009-01-12 09:37:12 EST
Created attachment 328742 [details]
Replacement VNC plugin for Service Discovery Applet

This replaces vncviewer.py.in.  It autodetects which vnc viewer is available, and handles the difference in command line parameters including the port / screen difference between XVNC and Vinagre.
Comment 11 Tim Niemueller 2009-01-12 13:34:18 EST
I'm currently short in time. I still think the service-discovery-applet should be in Fedora. My time constraints (thesis) will not allow me to do this before March though. Either we wait or someone else takes over.
Comment 12 Till Maas 2009-04-18 10:50:06 EDT
Please clear NotReady from the whiteboard when this request is ready for a review.
Comment 13 Tim Niemueller 2011-04-21 18:14:59 EDT
I have lost interest in this, someone else can take over if interested.

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