This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 452395

Summary: Review Request: lxlauncher - Open source replacement for Asus Launcher of the EeePC
Product: [Fedora] Fedora Reporter: Christoph Wickert <cwickert>
Component: Package ReviewAssignee: Robert Scheck <redhat>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, ma, notting, redhat-bugzilla, sundaram
Target Milestone: ---Flags: redhat: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-07-26 20:36:45 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 505781    

Description Christoph Wickert 2008-06-21 22:17:48 EDT
Spec URL: http://cwickert.fedorapeople.org/review/lxlauncher.spec
SRPM URL: http://cwickert.fedorapeople.org/review/lxlauncher-0.2-1.fc10.src.rpm
Description: 
LXLauncher is designed as an open source replacement for the Asus Launcher
included in their EeePC. It is desktop-independent and follows 
freedesktop.org specs, so newly added applications will automatically show 
up in the launcher, and vice versa for the removed ones.
LXLauncher is part of LXDE, the Lightweight X11 Desktop Environment.
Comment 1 Christoph Wickert 2008-06-22 09:17:53 EDT
Robert, I'm CC'ing you beause I know you have an EeePC, so maybe you can help me
with this review... ;)
Comment 2 Rahul Sundaram 2008-07-20 21:58:42 EDT
OK  | MUST: rpmlint is clean
OK  | MUST: The package must be named according to the Package…
OK  | MUST: The spec file name must match the base package…
OK  | MUST: The package must meet the Packaging Guidelines…
OK  | MUST: The package must be licensed with a Fedora approved…
OK  | MUST: The License field in the package spec file must. 
OK  | MUST: The spec file must be written in American English.
OK  | MUST: The package must successfully compile and build…
OK  | MUST: All build dependencies must be listed…
OK  | MUST: The spec file MUST handle locales properly…
OK  | MUST: Every binary RPM package which stores shared…
OK  | MUST: If the package is designed to be relocatable…
OK  | MUST: A package must own all directories that it creates
OK  | MUST: A package must not contain any duplicate files 
OK  | MUST: Permissions on files must be set properly. 
OK  | MUST: Each package must have a %clean section
OK  | MUST: Each package must consistently use macros
OK  | MUST: The package must contain code, or permissible 
N/A | MUST: Large documentation files should go in a -doc 
OK  | MUST: If a package includes something as %doc…
N/A | MUST: Header files must be in a -devel package.
N/A | MUST: Static libraries must be in a -static package.
N/A | MUST: Packages containing pkgconfig(.pc) files must…
N/A | MUST: If a package contains library files with a suffix…
N/A | MUST: In the vast majority of cases, devel packages must…
OK  | MUST: Packages must NOT contain any .la libtool archives, 
NOK | MUST: Packages containing GUI applications must include…

Shouldn't there be a .desktop file? How am I suppose to test the functionality
of this software or launch it?

NOK  | MUST: Packages must not own files or directories already

Why are you claiming ownership of 
%{_datadir}/desktop-directories/*.directory?

These are owned by redhat-menus package already. 


OK  | MUST: At the beginning of %install, each package MUST…
OK  | MUST: All filenames in rpm packages must be valid UTF-8.
OK  | SHOULD: If the source package does not include license 
-   | SHOULD: The description and summary section … translations…
OK  | SHOULD: The package builds in mock
-   | SHOULD: The package builds on all supported architectures
OK  | SHOULD: The reviewer should test that the package…
N/A | SHOULD: If scriptlets are used, those scriptlets must be sane…
N/A | SHOULD: Subpackages other than devel should usually require base…
N/A | SHOULD: The placement of pkgconfig(.pc) files depends on…
OK | SHOULD: If the package has file dependencies outside of shortlist…
OK  | MUST: All build dependencies must be listed…

Robert, do you want to do the official review? 
Comment 3 Christoph Wickert 2008-07-21 01:35:38 EDT
(In reply to comment #2)

>
> Shouldn't there be a .desktop file? How am I suppose to test the functionality
> of this software or launch it?

By simply running it from a terminal?? Works fine in _all_ desktops I've seen.

lxlauncher is supposed to be started by lxsession or whatever and running all
the time. Think of something like xfdesktop, it has no launcher ether. I could
however add a launcher to /etc/xdg/autostart, but this should IMO be disabled by
default.

My plan is to add a special EeePC Session to lxsession-lite in which lxlauchner
will be started automatically.

> NOK  | MUST: Packages must not own files or directories already
> 
> Why are you claiming ownership of 
> %{_datadir}/desktop-directories/*.directory?
> 
> These are owned by redhat-menus package already. 

AFAICS I'm not owning any other files but my own, 
%{_datadir}/desktop-directories/*.directory seems correct:
$ rpm -qf /usr/share/desktop-directories/
filesystem-2.4.13-1.fc9.i386
gnome-menus-2.22.2-1.fc9.i386
$ rpm -ql lxlauncher
/usr/bin/lxlauncher
/usr/share/desktop-directories/Learn.directory
/usr/share/desktop-directories/Math.directory
/usr/share/desktop-directories/Play.directory
/usr/share/desktop-directories/Science.directory
/usr/share/desktop-directories/Work.directory
/usr/share/doc/lxlauncher-0.2
...

The package drops 5 more Menus in there and so far the names do not conflikt are
not in redhat-menus or in gnome-menus, so there is no collision. Nevertheless I
can install them prefixed with lxde-* or alike to avoid problems long term.
Comment 4 Robert Scheck 2008-07-21 02:27:58 EDT
> By simply running it from a terminal?? Works fine in _all_ desktops I've seen.
> 
> lxlauncher is supposed to be started by lxsession or whatever and running all
> the time. Think of something like xfdesktop, it has no launcher ether. I could
> however add a launcher to /etc/xdg/autostart, but this should IMO be disabled 
> by default.

I didn't yet have a look to xfdesktop or equivalent things, but from my current
point of view, I would agree with you.

> AFAICS I'm not owning any other files but my own, 
> %{_datadir}/desktop-directories/*.directory seems correct:

For me, it seems also correct what is currently owned. %{_datadir}/desktop-
directories/*.directory refers to all *.directory files coming from the "make 
install DESTDIR=$RPM_BUILD_ROOT". Rahul, please clarify.

> The package drops 5 more Menus in there and so far the names do not conflikt 
> are not in redhat-menus or in gnome-menus, so there is no collision. 
> Nevertheless I can install them prefixed with lxde-* or alike to avoid 
> problems long term.

I would see prefixing as optional SHOULD, this is IMHO up to you.
Comment 5 Rahul Sundaram 2008-07-21 13:46:00 EDT
I would prefer a launcher. I don't have a eeepc with me to test right now but
running it on the command seems to just hang in GNOME

# lxlauncher

I don't see anything after this. 

Yes, a prefix would be good to avoid conflicts. The ownership otherwise seems
correct. 

Robert Scheck. this review is still officially assigned to you. 

Comment 6 Rahul Sundaram 2008-07-21 18:34:06 EDT
Just a quick note that I figured out how lxlauncher works. 
Comment 7 Christoph Wickert 2008-07-24 11:34:29 EDT
So do you still think it needs a launcher? IMO a launcher in the menu is
nonsens, nobody will launch a lauchner from the menu just to launch another app.

The only thing that makes sense to me is /etc/xdg/autostart, but I'm not sure
how to handle that for all desktops/window managers, because some do support
/etc/xdg/autostart but do not implement "Hidden=true".
Comment 8 Robert Scheck 2008-07-24 17:43:06 EDT
Okay, let's do the official review: I checked all my points which are mostly 
equivalent to that ones already mentioned, so I'll not re-post my list. From 
my side I've no complaints at all.

> NOK | MUST: Packages containing GUI applications must include…

I agree with the requestor that no *.desktop is required, doesn't make sense
here. Maybe the hint for the autostart can be put into a README-FEDORA or a 
similar file, but per default enabled when maybe some/not all desktops/window 
managers are supporting it, is not ideal.

> NOK  | MUST: Packages must not own files or directories already

I can't see any issue here, the result in the built RPM package looks fine.


APPROVED.
Comment 9 Christoph Wickert 2008-07-25 15:07:51 EDT
New Package CVS Request
=======================
Package Name: lxlauncher
Short Description: Open source replacement for Asus Launcher of the EeePC
Owners: cwickert
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: yes
Comment 10 Kevin Fenzi 2008-07-26 10:04:51 EDT
cvs done.
Comment 11 Fedora Update System 2008-07-26 20:20:57 EDT
lxlauncher-0.2-1.fc9 has been submitted as an update for Fedora 9
Comment 12 Fedora Update System 2008-07-26 20:22:00 EDT
lxlauncher-0.2-1.fc8 has been submitted as an update for Fedora 8
Comment 13 Christoph Wickert 2008-07-26 20:36:45 EDT
Thanks for the review, Robert. Closing.
Comment 14 Fedora Update System 2008-07-30 16:01:09 EDT
lxlauncher-0.2-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2008-07-30 16:06:45 EDT
lxlauncher-0.2-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 16 Christoph Wickert 2009-05-22 21:46:09 EDT
Package Change Request
======================
Package Name: lxlauncher
New Branches: EL-4 EL-5
Owners: cwickert
Comment 17 Kevin Fenzi 2009-05-23 01:28:43 EDT
cvs done.