[Bug 751537] New: Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell
https://bugzilla.redhat.com/show_bug.cgi?id=751537
Summary: Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: hal8600@hotmail.it QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-wea...
SRPM URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-wea...
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.
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
M.M. hal8600@hotmail.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
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
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@gmail.com
--- Comment #1 from Richard Shaw hobbes1069@gmail.com 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
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 #2 from M.M. hal8600@hotmail.it 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.
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@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
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 #4 from M.M. hal8600@hotmail.it 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.
- 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.
- 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.
- 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-wea...
SRPM URL: http://dl.dropbox.com/u/39458594/Fedora%20packages/gnome-shell-extension-wea...
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 #5 from Richard Shaw hobbes1069@gmail.com 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
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
Parag AN(पराग) panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |panemade@gmail.com
--- Comment #6 from Parag AN(पराग) panemade@gmail.com 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
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
Elad Alfassa el.il@doom.co.il changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED CC| |el.il@doom.co.il Resolution| |WONTFIX Last Closed| |2011-11-07 03:01:39
--- Comment #7 from Elad Alfassa el.il@doom.co.il 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.
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 #8 from Elad Alfassa el.il@doom.co.il 2011-11-07 03:03:56 EST --- Perhaps someone should try getting this into rpmfusion-free...
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 #9 from Mattia Meneguzzo hal8600@hotmail.it 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.
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 #10 from Mattia Meneguzzo hal8600@hotmail.it 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
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
Michal Jaegermann michal@harddata.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |michal@harddata.com
--- Comment #11 from Michal Jaegermann michal@harddata.com 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.
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 #12 from Elad Alfassa el.il@doom.co.il 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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=751537
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ssabchew@yahoo.com
--- Comment #13 from Jason Tibbitts tibbs@math.uh.edu --- *** Bug 822466 has been marked as a duplicate of this bug. ***
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=751537
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla