[Bug 751537] Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell

[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.


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

--- Comment #3 from Richard Shaw <hobbes1069 at gmail.com> 2011-11-05 13:10:09 EDT ---
Ok, unofficial review since I can't sponsor you.

1. Always run rpmlint on the SRPM (and RPMs if you build them).

I got the following output:

$ rpmlint
rpmbuild/gnome-shell-extension-weather/RPMS/noarch/gnome-shell-extension-weather-0-0.1.git4681a05.fc15.noarch.rpm 
gnome-shell-extension-weather.noarch: E: description-line-too-long C
gnome-shell-extension-weather is a simple extension for displaying weather
notifications in GNOME Shell.
gnome-shell-extension-weather.noarch: E: description-line-too-long C Currently,
the weather report including forecast for today and tomorrow is fetched from
Yahoo! Weather.

Basically you need to break your lines at 80 characters in the %description.

2. The following are not needed in your spec file and can be removed. (I would
normally say, unless you going to build for EL5 or older versions of Fedora,
but since this is for F16+, it's a given):

rm -rf %{buildroot} in %install
%defattr(-,root,root,-) in %files

3. There is a basic configuration gui in the tarball that's not installed by
default. The files in the 3.0 branch are bad so I pulled them for Master for me
but since it appears you're only planning to build for F16+ you should be able
to use the ones in the tarball. I tried it out and it seemed to work. You still
have to look up the WOEID manually but it has radio buttons for the other
settings.

In %install I added:

install -D -pm 0755 weather-extension-configurator.py \
        %{buildroot}%{_bindir}/weather-extension-configurator.py

desktop-file-install \
        --dir=%{buildroot}%{_datadir}/applications \
        weather-extension-configurator.desktop

In %files I added:

%{_bindir}/weather-extension-configurator.py
%{_datadir}/applications/weather-extension-configurator.desktop


Richard

-- 
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.