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-networkmanagement - Plasmoid to control Network Manager
https://bugzilla.redhat.com/show_bug.cgi?id=484535
Summary: Review Request: kde-plasma-networkmanagement - Plasmoid to control Network Manager Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: mathstuf@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw... SRPM URL: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw... Description: Plasmoid for KDE4 to control Network Manger.
It still has some issues, but we (the KDE SIG) are looking to get it into Rawhide so it can get testing before the F11 beta and so we can evaluate it as a possible default for F11 if it matures in time, otherwise, we plan to at least offer it as an option. We also plan to push it out to F10 and possibly F9 once it is reliable enough for a non-default option.
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=484535
Milos Jakubicek xjakub@fi.muni.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |xjakub@fi.muni.cz AssignedTo|nobody@fedoraproject.org |xjakub@fi.muni.cz Flag| |fedora-review?
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=484535
Armin feng.shaun@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |feng.shaun@gmail.com
--- Comment #1 from Armin feng.shaun@gmail.com 2009-02-08 16:21:56 EDT --- there is an rpmlint issue with the src rpm:
kde-plasma-networkmanagement.src:25: W: unversioned-explicit-obsoletes kde-plasma-networkmanager 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
it's better if you specify the highest version of kde-plasma-networkmanager it obsoletes.
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=484535
--- Comment #2 from Armin feng.shaun@gmail.com 2009-02-08 16:23:08 EDT --- there is an rpmlint issue with the src rpm:
kde-plasma-networkmanagement.src:25: W: unversioned-explicit-obsoletes kde-plasma-networkmanager 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
it's better if you specify the highest version of kde-plasma-networkmanager it obsoletes.
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=484535
--- Comment #3 from Armin feng.shaun@gmail.com 2009-02-08 16:32:24 EDT --- kde-plasma-networkmanagement.x86_64: W: no-documentation kde-plasma-networkmanagement.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/NetworkManager-kde4.conf kde-plasma-networkmanagement.x86_64: W: obsolete-not-provided kde-plasma-networkmanager 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
There is no %doc (the TODO and DESIGN files could be added as %doc)
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=484535
--- Comment #4 from Armin feng.shaun@gmail.com 2009-02-08 16:38:27 EDT --- also, for debug-info, it's better if you chop the %description in two lines, rpmlint complains about this:
kde-plasma-networkmanagement-debuginfo.x86_64: E: description-line-too-long This package provides debug information for package kde-plasma-networkmanagement. kde-plasma-networkmanagement-debuginfo.x86_64: W: filename-too-long-for-joliet kde-plasma-networkmanagement-debuginfo-0.1-0.3.20090207svn.fc10.x86_64.rpm
and the filename is too long for all of them, but I don't think that would be a big issue.
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=484535
--- Comment #5 from Armin feng.shaun@gmail.com 2009-02-08 16:42:41 EDT --- otherwise, it builds fine on x86_64 =)
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=484535
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@tigcc.ticalc.org
--- Comment #6 from Kevin Kofler kevin@tigcc.ticalc.org 2009-02-08 16:49:25 EDT ---
kde-plasma-networkmanagement.src:25: W: unversioned-explicit-obsoletes
kde-plasma-networkmanager
That one ought to be fixed.
kde-plasma-networkmanagement.x86_64: W: no-documentation
Mostly harmless. Is there no COPYING? That's worth a complaint upstream. TODO and DESIGN may or may not be worth shipping.
kde-plasma-networkmanagement.x86_64: W: non-conffile-in-etc
/etc/dbus-1/system.d/NetworkManager-kde4.conf
That's the usual D-Bus "config files" which aren't really "config files". It could be marked %config, but I think it probably shouldn't, that file isn't really intended to be customized. The path is in /etc because that's where D-Bus expects those files to be.
kde-plasma-networkmanagement.x86_64: W: obsolete-not-provided
kde-plasma-networkmanager
Harmless, but I think adding the Provides can't hurt and it will make "yum install kde-plasma-networkmanager" work.
kde-plasma-networkmanagement-debuginfo.x86_64: E: description-line-too-long
This package provides debug information for package kde-plasma-networkmanagement.
The line exceeds 80 characters, it should be broken into 2 lines.
kde-plasma-networkmanagement-debuginfo.x86_64: W: filename-too-long-for-joliet
kde-plasma-networkmanagement-debuginfo-0.1-0.3.20090207svn.fc10.x86_64.rpm
Ignore that one. The name is as it has to be according to the guidelines, you can't shorten it.
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=484535
--- Comment #7 from Armin feng.shaun@gmail.com 2009-02-08 16:56:16 EDT ---
Mostly harmless. Is there no COPYING? That's worth a complaint upstream. TODO and DESIGN may or may not be worth shipping.
no, there isn't. But I think TODO and DESIGN should be part of %doc, as they are upstream. Someone might need it.
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=484535
--- Comment #8 from Kevin Kofler kevin@tigcc.ticalc.org 2009-02-08 18:06:43 EDT --- Actually the -debuginfo description is autogenerated, so that description-line-too-long warning can also be ignored.
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=484535
Milos Jakubicek xjakub@fi.muni.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #9 from Milos Jakubicek xjakub@fi.muni.cz 2009-02-08 20:04:20 EDT --- - regarding the rpmlint output, as discussed in previous comments -- please fix: kde-plasma-networkmanagement.src:25: W: unversioned-explicit-obsoletes kde-plasma-networkmanagement.x86_64: W: obsolete-not-provided kde-plasma-networkmanagement.x86_64: W: no-documentation (include the TODO and DESIGN file)
- use macros in %build and %files: %{cmake_kde4} .. -DDBUS_SYSTEM_POLICY_DIR=/etc/dbus-1/system.d /etc/dbus-1/system.d/NetworkManager-kde4.conf
/etc => %{_sysconfdir}
- remove unnecessary BR (main package) and R (for -devel subpackage): BR/R: NetworkManager-devel which is required by NetworkManager-glib-devel BR/R: kdelibs4-devel which is required by kdebase-workspace-devel
Requires(post): /sbin/ldconfig xdg-utils Requires(postun): /sbin/ldconfig xdg-utils (Those are taken by rpm automagically)
- please provide a more appropriate description than %{summary} for the main package.
- please provide a less general URL than just kde.org, e.g. http://websvn.kde.org/trunk/playground/base/plasma/applets/networkmanager/
- there is no licensing info besides the qt dual license and .desktop files; nor in an attached LICENSE file, nor in the source files -- that's bad. Please ask upstream to include a license file and to mention the license in the sources. You should also include a LICENSE.GPL and GPL_EXCEPTION.txt file as the qt license states.
- please fix the gcc flags, something like this before the make call should be enough:
find . -name "flags.make" -execdir sed -i -e 's/-fno-exceptions -fno-check-new -fno-common//' -e 's/-fno-threadsafe-statics -fvisibility=hidden -fvisibility-inlines-hidden//' -e 's/-ansi//' {} ;
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=484535
--- Comment #10 from Milos Jakubicek xjakub@fi.muni.cz 2009-02-08 20:41:52 EDT --- ad *-debuginfo.*: E: description-line-too-long: filed as 484616
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=484535
--- Comment #11 from Rex Dieter rdieter@math.unl.edu 2009-02-10 22:09:04 EDT --- imo, leave the gcc flags alone, this is how pretty much all of kde is built in fedora already. If you have specific concerns, then we could consider those and consult upstream, but that's outside the scope of this review.
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=484535
--- Comment #12 from Ben Boeckel mathstuf@gmail.com 2009-02-20 22:02:08 EDT --- Fixed SPEC: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw... Fixed SRPM: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw...
rpmlint says that the Provides: should be versioned. Not sure if I should, and if so, what version it should be. %{version}-%{release} I'd guess, if any.
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=484535
--- Comment #13 from Milos Jakubicek xjakub@fi.muni.cz 2009-02-21 21:35:25 EDT --- (In reply to comment #12)
rpmlint says that the Provides: should be versioned. Not sure if I should, and if so, what version it should be. %{version}-%{release} I'd guess, if any.
- Yes, it should and those Provides/Obsoletes statements must be placed also by the subpackages, see this draft for details: https://fedoraproject.org/wiki/Archive:PackagingDrafts/ProvidesObsoletes
- remove duplicate Requires: openvpn/vpnc, they're again pulled by the NetworkManager-openvpn/vpnc packages.
- by providing a more appropriate description, I really didn't mean to expand the macro;) You should briefly describe the functions of the plasmoid, please try to do so for a few lines.
- regardins licensing it's a big mess:
GPLv2+: - libs/dbus/*
LGPLv2: - in libs/ui: accesspoint.cpp,h apitemdelegate.cpp,h apitemmodel.cpp,h apitemview.cpp,h ifaceitemmodel.cpp,h scanwidget.cpp,h - in libs: marshalarguments.h types.h - in libs/storage/settings: wephash.cpp,h - in settings: ip4config.cpp,h - in settings/service: busconnection.cpp,h networksettings.cpp,h
LGPLv2+: - in tests: testconfigxml.cpp,h testnewstorage.cpp,h
GPLv2 or GPLv3 ("Qt Nokia"): tests/qdbusfornm.cpp
MISSING LICENSE: libs/ui/vpnuiplugin.cpp
all others under libs/ are LGPLv2+ all others are GPLv2+
So...this is a bit problem, please:
- you can (but not must of course) try to persuade upstream to unify the licenses to GPLv2+ and LGPLv2+ if it is possible. Currently it violates the GPL licenses according to: https://fedoraproject.org/wiki/Licensing#GPL_Compatibility_Matrix (You cannot copy LGPLv2 code within GPLv2+ project without converting to GPLv2) So you should at least try that.
- you really should persuade them to: -- add the license info to libs/ui/vpnuiplugin.cpp if it not autogenerated -- include the license files for GPLv2 and LGPLv2, LICENSE.GPL and GPL_EXCEPTIONS.TXT. Suggest naming the GPLv2 license file as LICENSE.GPL to avoid duplicates. - please put the information I've gathered above into a LICENSING.INFO file and package it as a %doc
As it is now, the License tag would be: (GPLv2 or GPLv3) and GPLv2+ and LGPLv2+ and LGPLv2
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=484535
--- Comment #14 from Kevin Kofler kevin@tigcc.ticalc.org 2009-02-21 21:55:54 EDT --- As discussed on IRC, using the LGPLv2 code there is not a violation.
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=484535
--- Comment #15 from Ben Boeckel mathstuf@gmail.com 2009-02-23 20:17:45 EDT --- Fixed SPEC: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw... Fixed SRPM: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw...
Fixed the licensing line, removed dupe R a, fixed P/O for subpackages and got a better summary/description.
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=484535
--- Comment #16 from Milos Jakubicek xjakub@fi.muni.cz 2009-02-24 15:18:47 EDT ---
rpmlint -i ../RPMS/x86_64/kde-plasma-networkmanagement-*0.5*
[...] kde-plasma-networkmanagement-devel.x86_64: W: tag-in-description Obsoletes: Something that looks like a tag was found in the package's description. This may indicate a problem where the tag was not actually parsed as a tag but just textual description content, thus being a no-op. Verify if this is the case, and move the tag to a place in the specfile where %description won't fool the specfile parser, and rebuild the package. [...]
The P/O for subpackages are wrong placed, they must be before the %description otherwise they're taken as part of the %description as described in the rpmlint warning.
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=484535
--- Comment #17 from Ben Boeckel mathstuf@gmail.com 2009-02-25 22:24:14 EDT --- Fixed SPEC: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw... Fixed SRPM: http://benboeckel.net/packaging/kde-plasma-networkmanagement/kde-plasma-netw...
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=484535
Milos Jakubicek xjakub@fi.muni.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #18 from Milos Jakubicek xjakub@fi.muni.cz 2009-02-27 10:10:06 EDT --- OK, there are some minor outstanding issues regarding licensing, but none of them are blocking and can be done later on, otherwise all the problems have been addressed, this package is APPROVED.
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=484535
Ben Boeckel mathstuf@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #19 from Ben Boeckel mathstuf@gmail.com 2009-02-27 11:12:59 EDT --- New Package CVS Request ======================= Package Name: kde-plasma-networkmanagement Short Description: NetworkManager KDE 4 integration Owners: mathstuf rdieter Branches: F-9 F-10 InitialCC:
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=484535
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #20 from Kevin Fenzi kevin@tummy.com 2009-02-28 19:19:35 EDT --- cvs done.
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=484535
Ben Boeckel mathstuf@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
--- Comment #21 from Ben Boeckel mathstuf@gmail.com 2009-03-03 12:01:21 EDT --- Built for rawhide, closing.
package-review@lists.fedoraproject.org