Bug 751537
Summary: | Review Request: gnome-shell-extension-weather - An extension for displaying weather notifications in GNOME Shell | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mattia M. <mattia.meneguzzo+fedora> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | alex, elad, hobbes1069, michal, mishu, notting, package-review, panemade, robatino, ssabchew, theo148 |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-11-07 08:01:39 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Mattia M.
2011-11-05 10:45:26 UTC
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 (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. 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 (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 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 I saw this review request on devel list and remembered about following https://bugzilla.redhat.com/show_bug.cgi?id=718242#c1 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 Perhaps someone should try getting this into rpmfusion-free... (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. Here is the review request for the inclusion of the package in RPMFusion: https://bugzilla.rpmfusion.org/show_bug.cgi?id=2017 (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. (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 *** Bug 822466 has been marked as a duplicate of this bug. *** |