Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
Summary: Review Request: lxpanel - A lightweight X11 desktop panel Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: fedora@deadbabylon.de QAContact: fedora-package-review@redhat.com
Spec URL: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel.spec SRPM URL: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel-0.2.4-2.src.rpm Description: lxpanel is a lightweight X11 desktop panel. It works with any ICCCM / NETWM compliant window manager (eg sawfish, metacity, xfwm4, kwin) and features a tasklist, pager, launchbar, clock, menu and sytray.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
fedora@christoph-wickert.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@christoph-wickert.de, | |candyz0416@gmail.com
------- Additional Comments From fedora@christoph-wickert.de 2006-12-16 21:31 EST ------- (In reply to comment #0)
SRPM URL: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel-0.2.4-2.src.rpm
Error 404.
I took your spec and built the package, but there are some things it don't like:
- Source0: should not use a mirror, instead use dl.sourceforge.net/sourceforge/lxde/%{name}-%{version}.tar.gz
- BR startup-notification-devel?
- In the default config two starters (firefox and pcman-fm) are not working. Please take a look at Chung-Yen's package at ftp://cle.linux.org.tw/pub/fedora/cle/6/SRPMS/lxpanel-0.2.4-1.fc6.src.rpm for a fix.
Some othe more thoughts: I think someone should also roll a pcman-fm package, we should package the whole lx-desktop in extras. I have an icewm package that I can offer. You and Chung-Yen should unite your powers on pcman-fm, lxpanel, lxsession and lxmusic. What do you think?
As long as we don't have no pcman-fm package I suggest this starter should be removed from the default config.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@deadbabylon.de 2006-12-17 05:05 EST ------- (In reply to comment #1)
(In reply to comment #0)
SRPM URL:
http://deadbabylon.de/fedora/extras/lxpanel/lxpanel-0.2.4-2.src.rpm
Error 404.
My fault. Fixed it.
I took your spec and built the package, but there are some things it don't
like:
- Source0: should not use a mirror, instead use
dl.sourceforge.net/sourceforge/lxde/%{name}-%{version}.tar.gz
Corrected.
- BR startup-notification-devel?
Added.
- In the default config two starters (firefox and pcman-fm) are not working.
Please take a look at Chung-Yen's package at ftp://cle.linux.org.tw/pub/fedora/cle/6/SRPMS/lxpanel-0.2.4-1.fc6.src.rpm for a fix.
Added the patch.
Some othe more thoughts: I think someone should also roll a pcman-fm
package, we
should package the whole lx-desktop in extras. I have an icewm package that
I
can offer. You and Chung-Yen should unite your powers on pcman-fm, lxpanel, lxsession and lxmusic. What do you think?
I didn't know that someone else ist working on it. I will ask him what he would think. Package the whole LXDE-Desktop could be interesting for others. I only use lxpanel for myself and have never used lxmusic. Will have a look on it.
As long as we don't have no pcman-fm package I suggest this starter should
be
removed from the default config.
At first I've thought to replace pcmanfm.desktop with gnome-nautilus-home.desktop. But perhaps this could irritate some users when updating the package later and replace it again with pcmanfm.desktop.
For the moment: SPEC Url: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel.spec SRPM Url: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel-0.2.4-3.src.rpm
Changelog: - BR: startup-notification-devel - Added Patch1 from Chung-Yen to fix wrong starters in default config
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
fedora@christoph-wickert.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |fedora@christoph-wickert.de OtherBugsDependingO|163776 |163778 nThis| |
------- Additional Comments From fedora@christoph-wickert.de 2007-01-02 20:33 EST ------- Sebastian, have you heard something from Chung-Yen? If not, I'm going to review this package tomorrow.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@deadbabylon.de 2007-01-03 06:20 EST ------- (In reply to comment #3)
Sebastian, have you heard something from Chung-Yen? If not, I'm going to review this package tomorrow.
Sadly, I've heard nothing.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From candyz0416@gmail.com 2007-01-03 08:05 EST ------- lxmusic needs xmms2 (but xmms2 is not included in FE), so I prefer not to include lxmusic lxsession seems not work in FC6 (I have tried it before) So, I think only lxpanel and pcmanfm in Extras is enough.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@christoph-wickert.de 2007-01-03 08:33 EST ------- (In reply to comment #5)
lxmusic needs xmms2 (but xmms2 is not included in FE), so I prefer not to
include lxmusic
Yeah, I already tried to patch it for bmp/audacious, but it was too much work for only a tiny program.
lxsession seems not work in FC6 (I have tried it before) So, I think only lxpanel and pcmanfm in Extras is enough.
So is it ok for you if Sebastian takes over lxpanel? You could submit your pcmanfm package for review then, if you like.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
fedora@christoph-wickert.de changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778 |163779 nThis| |
------- Additional Comments From fedora@christoph-wickert.de 2007-01-06 12:41 EST ------- REVIEW for 80f9ae6864029fd3b0635e71f486c538 lxpanel-0.2.4-3.src.rpm
OK - rpmlint -i lxpanel-0.2.4-3.src.rpm W: lxpanel non-coherent-filename lxpanel-0.2.4-3.src.rpm The file which contains the package should be named <NAME>-<VERSION>-<RELEASE>.<ARCH>.rpm.
This is odd, I don't see no error here. Something ether is wrong with rpmlint or with your build environment. I don't see this error when I rebuild the srpm locally or in mock, so I choose to ignore it in this review, but can you send me your .rpmmarcos file so we can investigate this further? Does the error go away, when you rebuild the package once again locally?
OK - package and spec named according to the package naming guidelines OK - package meets packaging guidelines OK - license is open-source compatible, but COPYING looks dual licensed for me. First part is a BSD like license, second GPLv2. OK - Since GPL is more restrictive than BSD the whole package becomes GPL. So the license field in the spec is ok. OK - COPYING included in source and correctly installed in %doc OK - spec is in American English OK - spec is legible
MINOR NOTE - line warps in long fields like %description are usually done after 79 characters.
OK - source in srpm matches upstream by md5 37d0e9f2993fc63d9e7e1684552e10b4 OK - package compiles and build into binaries on core 6 i386 OK - no known ExcludeArchs OK - BuildRequires sane OK - locales handled correctly OK - no shared libs to worry about OK - package is not relocatable OK - package owns all directories it creates
MINOR NOTE - Instead of %dir %{_datadir}/lxpanel/ %{_datadir}/lxpanel/* %dir %{_libdir}/lxpanel/ %{_libdir}/lxpanel/* you could simply use %{_datadir}/lxpanel/ %{_libdir}/lxpanel/ since you include everything in these directories anyway. The slashes at the end of the line is only for humans to indicate its a dir.
OK - no duplicates in %files OK - file permissions and %defattr correct OK - valid clean section OK - macro usage consistent OK - code, not content OK - no large docs OK - docs don't affect runtime OK - no header files, static libs or *.pc files OK - no libtool archives OK - IMO no desktop file is needed since it's panel and not what I call a typical program/standalone application. OK - package doesn't own directories already owned by other files OK - package builds in mock (devel) OK - lxpanel works fine, but lxpanelctl is buggy. I can't add more starter because the "Select Application"-Dialog doesn't list the files in /usr/share/applications. Also hitting return in the location bar doesn't work. Looking at src/plugins/launchbar.c I think this is a known issue (see the FIXME in line 490) and isn't really meant to work atm.
So from a reviewers point of view everything looks fine except rpmlint. If the rpmlint error on the srpm can be fixed with a simple rebuild build you can consider this package to be APPROVED.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@deadbabylon.de 2007-01-08 15:08 EST ------- (In reply to comment #7)
OK - rpmlint -i lxpanel-0.2.4-3.src.rpm W: lxpanel non-coherent-filename lxpanel-0.2.4-3.src.rpm The file which contains the package should be named <NAME>-<VERSION>-<RELEASE>.<ARCH>.rpm.
Mhh. Strange. I can't remember which .rpmmacros I have used for this package. So I've uploaded a new version which is the result of the mock build. Could you test it for this issue (just to play safe this time)?
First part is a BSD like license, second GPLv2. OK - Since GPL is more restrictive than BSD the whole package becomes GPL. So the license field in the spec is ok.
The GPL is also the license at gnomefiles.org: http://www.gnomefiles.org/app.php/LXPanel
MINOR NOTE - line warps in long fields like %description are usually done
after
79 characters.
Fixed.
MINOR NOTE - Instead of %dir %{_datadir}/lxpanel/ %{_datadir}/lxpanel/* %dir %{_libdir}/lxpanel/ %{_libdir}/lxpanel/* you could simply use %{_datadir}/lxpanel/ %{_libdir}/lxpanel/
Ok. Fixed.
OK - IMO no desktop file is needed since it's panel and not what I call a typical program/standalone application.
Also think so. gnome-panel and kicker also have no desktop file.
OK - lxpanel works fine, but lxpanelctl is buggy. I can't add more starter because the "Select Application"-Dialog doesn't list the files in /usr/share/applications. Also hitting return in the location bar doesn't
work.
Looking at src/plugins/launchbar.c I think this is a known issue (see the
FIXME
in line 490) and isn't really meant to work atm.
You're right. Seems to be already filed as a bug: http://sourceforge.net/tracker/index.php?func=detail&aid=1623222&gro...
SPEC Url: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel.spec SRPM Url: http://deadbabylon.de/fedora/extras/lxpanel/lxpanel-0.2.4-4.fc6.src.rpm
Changelog: * Mon Jan 08 2007 Sebastian Vahl fedora@deadbabylon.de - 0.2.4-4 - Fixed some minor issues from the review process (#219930)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@christoph-wickert.de 2007-01-08 15:48 EST ------- (In reply to comment #8)
So I've uploaded a new version which is the result of the mock build. Could you test it for this issue (just to play safe this time)?
Looks fine, ACCEPTED.
Don't forget to change the default starters patch once we have a pcmanfm package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
fedora@deadbabylon.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From candyz0416@gmail.com 2007-01-11 20:44 EST ------- One thine you may need to know: the lxde author says he will no more develop and maintain these programs
So there is no NEXT version of lxde. That is one reason why I don't want to maintain any of lxde packages.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219930
------- Additional Comments From fedora@deadbabylon.de 2007-01-13 14:49 EST ------- (In reply to comment #10)
One thine you may need to know: the lxde author says he will no more develop and maintain these programs
So there is no NEXT version of lxde. That is one reason why I don't want to maintain any of lxde packages.
Mhh. Not good. Didn't know that. I've written to the maintainer of lxpanel and asked him about that. After I get an answer I will decide what to do with lxpanel in extras.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: lxpanel - A lightweight X11 desktop panel
https://bugzilla.redhat.com/show_bug.cgi?id=219930
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide
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=219930
Christoph Wickert fedora@christoph-wickert.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|163779(FE-ACCEPT) |505781(LXDE)
package-review@lists.fedoraproject.org