[Bug 450539] Review Request: service-discovery-applet - Service discovery applet based on Avahi for the Gnome panel

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: service-discovery-applet - Service discovery applet based on Avahi for the Gnome panel


https://bugzilla.redhat.com/show_bug.cgi?id=450539





------- Additional Comments From fedora at christoph-wickert.de  2008-06-30 19:51 EST -------
(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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.