[Bug 209614] Review Request: wmmemload - windowmaker dock app

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

Summary: Review Request: wmmemload - windowmaker dock app


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=209614





------- Additional Comments From dkovalsk at redhat.com  2006-10-11 17:28 EST -------
(In reply to comment #2)
> I don't have access to the .src.rpm.
Sorry, the permissions got messed up while copying


> * I would personally have dropped 'for window managers such as WindowMaker'
>   from the summary, since it will be shorter, and also those apps 
>   are well suited to fluxbox, for example if I'm not wrong.
Agreed, fixed


> * the provide wmmemload is unusefull, it is automatically set by rpm
removed


> * The Epoch is not needed. In my opinion it is clearer if it is not 
>   mentioned when set to 0
removed


> * the buildroot is not the preferred one
>
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1
>   (although it has been agreed that %(%{__id_u} -n) could be removed).
not much of a difference, but no proble to fix :-)



> * The Requires are not needed, there are picked up automatically by rpm
oh, didn't know about that, thanks for the info, fixed



> * BuildRequires: libX11-devel is optional since libX11-devel is required
>   by libXext-devel or libXpm-devel
aaah, ok, removed


> * export CFLAGS="$RPM_OPT_FLAGS" is unneeded, it is part of %configure
removed


> * if I'm not wrong, prefixing with %{_builddir}/%{name}-%{version}/ is 
>   not needed in %install since it is what the current working directory 
>   is set to.
ok, removed



> * in %files, I think it is better to use
> %{_mandir}/man1/wmmemload.1*
> instead of
> %{_mandir}/man1/wmmemload.1.gz
>   to catch no compression and different compression schemes.
looking good to me, modified


> * in the changelog, I think the 0: corresponding with epoch is unneeded
fixed :-)

still have to upload the files together with the other package, I'll add another
comment when I do that tomorrow morning


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.