Bug 751537 - Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell
Review Request: gnome-shell-extension-weather - An extension for displaying w...
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 822466 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-05 06:45 EDT by Mattia M.
Modified: 2013-04-25 17:47 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-11-07 03:01:39 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Mattia M. 2011-11-05 06:45:26 EDT
Spec URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-weather/gnome-shell-extension-weather.spec

SRPM URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-weather/gnome-shell-extension-weather-0-0.1.git4681a05.fc16.src.rpm

Description:
gnome-shell-extension-weather is a simple extension for displaying weather notifications in GNOME Shell. Currently, the weather report including forecast for today and tomorrow is fetched from Yahoo! Weather.
This is my first package and I am seeking a sponsor.
Comment 1 Richard Shaw 2011-11-05 09:55:32 EDT
I actually packaged this for my own use back when F15 was released. The only reason I never submitted it was that the weather location had to be determined and set in a rather manual way. Has that been resolved?

If so I'd be willing to take this one but I'm not a sponsor yet.

Richard
Comment 2 Mattia M. 2011-11-05 11:53:09 EDT
(In reply to comment #1)
> I actually packaged this for my own use back when F15 was released. The only
> reason I never submitted it was that the weather location had to be determined
> and set in a rather manual way. Has that been resolved?
> 
> If so I'd be willing to take this one but I'm not a sponsor yet.
> 
> Richard

The settings (location, temperature units etc.) still have to be set through command line, as explained on the page https://github.com/simon04/gnome-shell-extension-weather under "Configuration".
But in my opinion this is not an issue at all.
Comment 3 Richard Shaw 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
Comment 4 Mattia M. 2011-11-06 04:47:17 EST
(In reply to comment #3)

Thanks for your review and precious suggestions.


> Ok, unofficial review since I can't sponsor you.
> 
> 1. Always run rpmlint on the SRPM (and RPMs if you build them).


Ops, I forgot that.


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


Done.

 
> 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


Removed.

 
> 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


Added.
I also added "python" to the "Requires:" section and the commands needed to compile the GSettings schemas (as suggested on the Fedora Wiki page http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GSettings_Schema ).

Now rpmlint, executed on the .spec, the .src.rpm and the .rpm, returns:

gnome-shell-extension-weather.noarch: W: no-manual-page-for-binary weather-extension-configurator.py
2 packages and 1 specfiles checked; 0 errors, 1 warnings.


Here are the links to the new SPEC and SRPM files:

Spec URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-weather_2/gnome-shell-extension-weather.spec

SRPM URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-weather_2/gnome-shell-extension-weather-0-0.1.git1bb2dd6.fc16.src.rpm
Comment 5 Richard Shaw 2011-11-06 11:49:23 EST
Yup, I realized it needed "Requires: python" after my last post, good catch.

I think you're in pretty good shape. Now you just need to get sponsored. 

A couple of ways to do that. You can package more software, especially stuff off the wishlist[1]. You can also perform unofficial reviews other review requests[2]. Be sure to state that it is in fact an unofficial review and that you're seeking sponsorship.

Richard

[1] http://fedoraproject.org/wiki/Package_maintainers_wishlist
[2] http://fedoraproject.org/PackageReviewStatus/NEW.html
Comment 6 Parag AN(पराग) 2011-11-07 00:07:11 EST
I saw this review request on devel list and remembered about following
https://bugzilla.redhat.com/show_bug.cgi?id=718242#c1
Comment 7 Elad Alfassa 2011-11-07 03:01:39 EST
Sorry, this package can not be included in Fedora.
from my response to bug #718242:

Sorry, but we asked fedora legal long ago, and, well, this extension can't be
included in fedora due to the license terms of the yahoo api.
see http://lists.fedoraproject.org/pipermail/legal/2011-May/001633.html for
more information.




-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 8 Elad Alfassa 2011-11-07 03:03:56 EST
Perhaps someone should try getting this into rpmfusion-free...
Comment 9 Mattia M. 2011-11-07 03:34:45 EST
(In reply to comment #8)
> Perhaps someone should try getting this into rpmfusion-free...

OK, I'll try to ask for its inclusion in RPMFusion.
Comment 10 Mattia M. 2011-11-07 10:31:04 EST
Here is the review request for the inclusion of the package in RPMFusion: https://bugzilla.rpmfusion.org/show_bug.cgi?id=2017
Comment 11 Michal Jaegermann 2011-12-12 14:59:27 EST
(In reply to comment #9)
> (In reply to comment #8)
> > Perhaps someone should try getting this into rpmfusion-free...
> 
> OK, I'll try to ask for its inclusion in RPMFusion.

Regardless of an admisibility this package to Fedora it should be noted that an included 'weather-extension-configurator.py' is broken as it does not allow to change what it calls "woeid".  Regardless of what you type in a corresponding form field it has no effect.  Moreover despite of consistently calling it that way it expects no WOEID, like returned for example by http://woeid.rosselliot.co.nz/, but some other location code one has to divine somehow to change from a deafault Insbruck or to adjust to a new location.  Currently possible using 'gsettings' once you found such code.
Comment 12 Elad Alfassa 2011-12-12 15:02:36 EST
(In reply to comment #11)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Perhaps someone should try getting this into rpmfusion-free...
> > 
> > OK, I'll try to ask for its inclusion in RPMFusion.
> 
> Regardless of an admisibility this package to Fedora it should be noted that an
> included 'weather-extension-configurator.py' is broken as it does not allow to
> change what it calls "woeid".  Regardless of what you type in a corresponding
> form field it has no effect.  Moreover despite of consistently calling it that
> way it expects no WOEID, like returned for example by
> http://woeid.rosselliot.co.nz/, but some other location code one has to divine
> somehow to change from a deafault Insbruck or to adjust to a new location. 
> Currently possible using 'gsettings' once you found such code.

This is not the place to report this problem. Report it to the upstream maintainer.




-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 13 Jason Tibbitts 2013-04-24 14:35:04 EDT
*** Bug 822466 has been marked as a duplicate of this bug. ***

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