[Bug 483187] New: Review Request: kde-plasma-weather - Plasma applet for weather forecasts - package-review

[Bug 483187] New: Review Request: kde-plasma-weather - Plasma applet for weather forecasts

[Bug 225630] New: Merge Review:...

[Bug 483729] New: Review Request:...

Red Hat Bugzilla

Thursday, 29 January 2009 Thu, 29 Jan '09

11:18 p.m.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. Summary: Review Request: kde-plasma-weather - Plasma applet for weather forecasts https://bugzilla.redhat.com/show_bug.cgi?id=483187 Summary: Review Request: kde-plasma-weather - Plasma applet for weather forecasts Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody(a)fedoraproject.org ReportedBy: kevin(a)tigcc.ticalc.org QAContact: extras-qa(a)fedoraproject.org CC: notting(a)redhat.com, fedora-package-review(a)redhat.com Estimated Hours: 0.0 Classification: Fedora Spec URL: http://repo.calcforge.org/f10/kde-plasma-weather.spec SRPM URL: http://repo.calcforge.org/f10/kde-plasma-weather-0.0-0.2.fc9.20090130svn.... Description: A plasmoid showing weather reports and forecasts. Note: You need KDE 4.2 to build and/or test this. Scratch build for Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=1092688 -- 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.

Show replies by date

Red Hat Bugzilla

Sunday, 1 February Sun, 1 Feb

12:31 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Orcan 'oget' Ogetbil <oget.fedora(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oget.fedora(a)gmail.com --- Comment #1 from Orcan 'oget' Ogetbil <oget.fedora(a)gmail.com> 2009-02-01 01:31:14 EDT --- Here are my notes: ! rpmlint says: kde-plasma-weather.x86_64: W: no-documentation but there is really no documentation, so this can be ignored ? The package must be named according to the Package Naming Guidelines. Upstream webpage says: "For those looking for the weather forecast plasmoid (that's the official name of it), it's now..." So shall we call this package "kde-plasma-weatherforecast" instead? * please make use of the %{name} macro. * According to the guidelines http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_pack... the pre-release packages are versioned in a special way. Check the "kismet" example given in the above link. So it would probably better to change Version: 0.0 Release: 0.2%{?dist}.20090130svn to Version: 0 Release: 0.2.20090130svn%{?dist} The right-hand side of the disttag is to be used when there is a fix in an older branch, e.g. F-9 * Isn't plasma-devel provided by kdelibs-devel which is one of the dependencies of kdebase-workspace-devel? So, that BR seems redundant. * Fedora-specific flag -fexceptions is overriden by -fno-exceptions but I think this is a problem with the compilation of kde itself. Can this be fixed? * Do we really need call ldconfig on post{,un} ? -- 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.

Red Hat Bugzilla

7:52 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

So shall we call this package "kde-plasma-weatherforecast" instead?

It's under extragear/base/plasma/applets/weather in KDE SVN, that's where the name "weather" comes from.

...

* please make use of the %{name} macro.

What for? It's not like the name is going to change frequently. That's just useless use of macros which doesn't help anyone.

...

Version: 0

I thought the recommended version to use for checkouts without a version was 0.0, maybe it changed or maybe I just remembered wrong. (I see they use just 0 in Packaging:NamingGuidelines.) But if I change it from 0.0 to 0 now, this will break the upgrade path for those who already installed the package from kde-redhat testing. 0.0 will upgrade just fine to any version number they'll actually use.

...

Release: 0.2.20090130svn%{?dist}

OK, I can put the %{?dist} at the end, not sure why I put it before, probably just a typo. (Not that the guidelines clearly say that 0.2%{?dist}.20090130svn is invalid, but I agree that 0.2.20090130svn%{?dist} makes more sense.)

...

* Isn't plasma-devel provided by kdelibs-devel which is one of the dependencies of kdebase-workspace-devel? So, that BR seems redundant.

What if we make a separate plasma-devel one day? But those "redundant" BRs have also been removed from other packages so I can remove it from this one too.

...

* Fedora-specific flag -fexceptions is overriden by -fno-exceptions but I think this is a problem with the compilation of kde itself. Can this be fixed?

It's a KDE "feature". I can bring this up at the next KDE SIG meeting. But does it really matter for this package? It makes sense for a library

...

* Do we really need call ldconfig on post{,un} ?

Good question. I think we don't, as there's no public shared library, only a plugin in %{_kde4_libdir}/kde4. I'll remove it. -- 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.

Red Hat Bugzilla

7:54 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 --- Comment #3 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 08:54:39 EDT --- Actually, I'll replace Version: 0.0 with a real version number, I just asked the upstream author what to use. I also asked him whether he prefers kde-plasma-weather or kde-plasma-weatherforecast. -- 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.

Red Hat Bugzilla

7:59 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 --- Comment #4 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 08:59:17 EDT --- Oh, I didn't finish my sentence: I was saying: it makes sense for a library to require exception-safety (and thus -fexceptions) because users of the library may want to use exceptions, but for an application or a plasmoid, it's pretty much irrelevant. And in any case it's not an issue with this package, but with the general KDE default flags. -- 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.

Red Hat Bugzilla

8:06 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Kevin Kofler <kevin(a)tigcc.ticalc.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |shawn.starr(a)rogers.com Flag| |needinfo?(shawn.starr@roger | |s.com) --- Comment #5 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 09:06:15 EDT --- Waiting for feedback from upstream: <Kevin_Kofler> spstarr_work: What's the official version number of the weather forecast plasmoid? <Kevin_Kofler> 1.0 prerelease (i.e. 1.0-0.1.2009xxxxsvn)? <Kevin_Kofler> And should it be called kde-plasma-weather or kde-plasma-weatherforecast? -- 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.

Red Hat Bugzilla

11:51 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

> * please make use of the %{name} macro. What for? It's not like the name is going to change frequently. That's just useless use of macros which doesn't help anyone.

I've been told in many reviews that %{name} should be used extensively, except a couple places like the URL. Even now, since there's a possibility that the package name may be changed, it might come handy. (In reply to comment #3)

...

Actually, I'll replace Version: 0.0 with a real version number, I just asked the upstream author what to use. I also asked him whether he prefers kde-plasma-weather or kde-plasma-weatherforecast.

alright. let's wait. -- 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.

Red Hat Bugzilla

12:01 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

I've been told in many reviews that %{name} should be used extensively

Did this advice have basis in fedora's packaging guidelines? If so, where? All I'm aware of, is that macros should be used consistently, ie, avoid using %{name} in some places and hard-code it others. -- 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.

Red Hat Bugzilla

12:20 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

> I've been told in many reviews that %{name} should be used extensively Did this advice have basis in fedora's packaging guidelines? If so, where?

I'm not 100% sure that it is in the guidelines (I never found it). But I have been told this so many times by different reviewers, that I took it as an unwritten convention, or maybe it is so deeply hidden inside the guidelines that nobody remembers where it is... or it is just a legend. If it is an unwritten convention, sure, you don't have to obey it. But it would be nice if you do. side note: Another thing that I've been told many times to use in a very specific unique way and that I couldn't find in the guidelines is %defattr(-,root,root,-)

...

All I'm aware of, is that macros should be used consistently, ie, avoid using %{name} in some places and hard-code it others.

Well, strictly speaking, since %{name} has to be used in the BuildRoot, any other usage will cause an inconsistency. -- 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.

Red Hat Bugzilla

12:31 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Shawn Starr <shawn.starr(a)rogers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(shawn.starr@roger | |s.com) | --- Comment #9 from Shawn Starr <shawn.starr(a)rogers.com> 2009-02-01 13:31:22 EDT --- Well, the plasmoid is "Weather Forecast" but we didn't change the .desktop file to reflect this. X-KDE-Library=plasma_applet_weather You could call the RPM kde-plasma-weatherforecast but the installed plasmoid is 'plasma_applet_weather' Version is '1.0' for KDE 4.2. -- 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.

Red Hat Bugzilla

12:46 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 --- Comment #10 from Orcan 'oget' Ogetbil <oget.fedora(a)gmail.com> 2009-02-01 13:46:11 EDT --- I'm not sure if this is related to packaging. The image on the left-hand side of the applet does not look right. No matter how the weather is, it shows me a large black Ø contained in a larger white rectangle. Is this what this plasmoid supposed to show? -- 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.

Red Hat Bugzilla

1:05 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

side note: Another thing that I've been told many times to use in a very specific unique way and that I couldn't find in the guidelines is %defattr(-,root,root,-)

That one is because the files must be owned by root. Without this, they can be owned by some other user depending on how the RPM package is built.

...

The image on the left-hand side of the applet does not look right. No matter how the weather is, it shows me a large black Ø contained in a larger white rectangle.

This is because the BBC reports the current weather conditions as N/A for several locations. Apparently they don't get reports of weather conditions, only temperature and wind. So we get a huge N/A icon. :-( -- 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.

Red Hat Bugzilla

1:12 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 --- Comment #12 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 14:12:58 EDT --- * Sun Feb 01 2009 Kevin Kofler <Kevin(a)tigcc.ticalc.org> 1.0-0.1.20090130svn - Set Version to 1.0 - Fix Release (put alphatag before disttag) - Remove redundant BR plasma-devel - Don't call ldconfig, not needed - Use %%{name} macro Spec URL: http://repo.calcforge.org/f10/kde-plasma-weather.spec New SRPM URL: http://repo.calcforge.org/f10/kde-plasma-weather-1.0-0.1.20090130svn.fc9.... -- 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.

Red Hat Bugzilla

1:25 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

...

> side note: Another thing that I've been told many times to use in a very > specific unique way and that I couldn't find in the guidelines is > %defattr(-,root,root,-) That one is because the files must be owned by root. Without this, they can be owned by some other user depending on how the RPM package is built.

I was more considered about the "-"s. I don't know why %defattr(-,root,root) or %defattr(0644,root,root,0755) is "bad" (even for RPMs for which one doesn't need to assign exotic permissions)

...

> The image on the left-hand side of the applet does not look right. No matter > how the weather is, it shows me a large black Ø contained in a larger white > rectangle. This is because the BBC reports the current weather conditions as N/A for several locations. Apparently they don't get reports of weather conditions, only temperature and wind. So we get a huge N/A icon. :-(

That's what I suspected. The weather in this s.hole where I reside is so cold that BBC doesn't know how to picture it. Anyways, the package is good to go now. (One last thing, just an idea to clear possible confusions in the future): you can add to the description that this plasmoid is planned to be included in KDE 4.3.) ----------------------------------------------------- This package (kde-plasma-weather) is APPROVED by oget ----------------------------------------------------- -- 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.

Red Hat Bugzilla

1:46 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Kevin Kofler <kevin(a)tigcc.ticalc.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs? --- Comment #14 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 14:46:37 EDT --- New Package CVS Request ======================= Package Name: kde-plasma-weather Short Description: Plasma applet for weather forecasts Owners: kkofler spstarr Branches: InitialCC: tuxbrewr -- 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.

Red Hat Bugzilla

1:48 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

Red Hat Bugzilla

1:49 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Kevin Kofler <kevin(a)tigcc.ticalc.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs? --- Comment #16 from Kevin Kofler <kevin(a)tigcc.ticalc.org> 2009-02-01 14:49:17 EDT --- New Package CVS Request ======================= Package Name: kde-plasma-weather Short Description: Plasma applet for weather forecasts Owners: kkofler spstarr Branches: F-9 F-10 InitialCC: tuxbrewr -- 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.

Red Hat Bugzilla

Monday, 2 February Mon, 2 Feb

10:10 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

Red Hat Bugzilla

Tuesday, 3 February Tue, 3 Feb

11:46 a.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 --- Comment #18 from Shawn Starr <shawn.starr(a)rogers.com> 2009-02-03 12:46:39 EDT --- The spec file will be changed today. I am getting an official extragear tarball instead of pulling from SVN. We will use this for 4.2.x series, 4.3 will be using kdeplasma-addons. -- 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.

Red Hat Bugzilla

1:11 p.m.

New subject: [Bug 483187] Review Request: kde-plasma-weather - Plasma applet for weather forecasts

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=483187 Shawn Starr <shawn.starr(a)rogers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE --- Comment #19 from Shawn Starr <shawn.starr(a)rogers.com> 2009-02-03 14:11:15 EDT --- This is now in the package conveyor belt for rawhide -- 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.

5589

days inactive

5593

days old

package-review@lists.fedoraproject.org

Manage subscription

19 comments

1 participants

tags (0)

participants (1)

Red Hat Bugzilla

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

[Bug 483187] New: Review Request: kde-plasma-weather - Plasma applet for weather forecasts