[Bug 576685] Review Request: pekwm - A small and flexible window manager

[bugzilla at redhat.com]

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=576685

--- Comment #5 from Germán Racca <gracca at gmail.com> 2010-04-20 16:39:19 EDT ---
Hi Christoph:

I'm very sorry for the delay in responding, I've been very busy at work but I'm
back again to work in Fedora rpms. Please find my updated files here:

Spec URL: http://sites.google.com/site/gracca/pekwm.spec
SRPM URL: http://sites.google.com/site/gracca/pekwm-0.1.12-1.src.rpm

> I have been packaging this today too before I found this review. Our specs are
> looking nearly the same which is a good sign.

That's great! Now I'm going to try to answer your comments.

> Some comments:

> FIX - not latest stable version: Please update the package to 0.1.12

OK, done (updated to version 0.1.12).

> - BuildRequrires: During configure you see some checks that are not fulfilled:
...
> This means you need add libICE-devel, libXpm-devel and libjpeg-devel

OK, done (added to BuildRequires).

> - touch %{buildroot}%{_datadir}/xsessions/%{name}.desktop is not needed

OK, done (removed).

> - xsession file should have Type=XSession instead of Type=Application

OK, done (changed to XSession type).

> - Add INSTALL='install -p' to the make install ... line to preserve timestamps

OK, done (added "install -p").

> - The timestamp of the source tarball should also match upstream, see above
> link for how to achieve this.

OK, done (downloaded with timestamping turned on in wget).

> - %{_datadir}/%{name}/ is not owned, only the files inside. Thus an empty
> folder will remain after uninstalling the package. Just drop the * at the end
> of that line.

OK, done (I've learned a lot whit this comment: Thanks!)

> - %{_sysconfdir}/%{name}/start should also be tagged as %config(noreplace). The warning of rpmlint can be ignored.

OK, done (tagged as config file).

> - ChangeLog.until-0.1.6 should be included in %doc, maybe also ChangeLog.aewm++

OK, done (both added).

> - I think the applications menu should be patched to not include apps that are
> not in Fedora, e.g mozilla-firefox is called firefox. At least Pine and
> StarOffice should not be in the menu since they are not free software. Use
> Alpine instead of Pine. There is a lot of room for improvement, add what you
> think makes sense.

OK, I've included a patch to take care of this. However, I need that you read
carefully the menu list for possible errors or different ideas/suggestions;
some apps I removed and some I replaced. A summary here:

vi: removed (vi is a symlink to vim)
MPlayer: substitued gmplayer by gnome-mplayer
Ogle: substitued by Totem
XCalc: substitued by gcalctool
XMan: removed
GGv: removed
Gkrellm2: removed
ROX Filer: removed
Mozilla: removed
Mozilla Firefox: removed
Pine: substitued by Alpine
Mozilla Thunderbird: removed
Gaim: substitued by Pidgin
StarOffice: removed

> - How about including the stuff from the contrib folder in doc? If you include
> a script in doc, make sure it is not executable.

Here I didn't understand very well. Why contrib applications in doc section?

Once again, thanks very much!
Germán.

-- 
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.