Bug 208422 - Review Request: wmctrl - A command line tool to interact with an EWMH/NetWM compatible X Window Manager.
Review Request: wmctrl - A command line tool to interact with an EWMH/NetWM ...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-28 10:36 EDT by Michael Rice
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-07 01:51:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Michael Rice 2006-09-28 10:36:58 EDT
Spec URL: http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl.spec
SRPM URL: http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl-1.07-1.fc5.src.rpm
Description: 
The wmctrl program is a UNIX/Linux command line tool to interact with an EWMH/NetWM compatible X Window Manager. The tool provides command line access to almost all the features defined in the EWMH specification. It can be used, for example, to obtain information about the window manager, to get a detailed list of desktops and managed windows, to switch and resize desktops, to make windows full-screen, always-above or sticky, and to activate, close, move, resize, maximize and minimize them. The command line access to these window management functions makes it easy to automate and execute them from any application that is able to run a command in response to an event
Comment 1 Patrice Dumas 2006-09-28 10:57:02 EDT
Is it your first package? In that case you should block
the FE-NEEDSPONSOR bug.

Some quick comments:

You should wrap your lines at 80 columns for %description

The summary is too long and should not end with a dot. Since
I think that most if not all the window managers in fedora are 
EWMH/NetWM compatible (except maybe mwm and twm...) I think 
it is not very usefull to specify that in the summary.

-n %{name}-%{version} is not usefull on the %setup line
since it is the default.

Application/Control don't seem to exist. The groups are at:
http://fedoraproject.org/wiki/RPMGroups
I think that
Group: User Interface/X
could be right.

libXmu-devel allready requires libX11-devel
Comment 2 Michael Rice 2006-09-28 13:20:23 EDT
This is my second package, and I am seeking a sponsor
Comment 3 Patrice Dumas 2006-09-28 13:55:17 EDT
Your bug should also block FE-NEW... I readd it.
Comment 4 Michael Rice 2006-09-28 14:02:11 EDT
Patrice, thanks for looking this over, pardon me if I get some things wrong with
this bugzilla, I have never used this before. FE-NEW Im not sure what you mean
by this.. I redid the spec file and built a new srpm it is here:
http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl-1.07-2.fc5.src.rpm
And the spec is here:
http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl-1.07-2.fc5/wmctrl.spec
Comment 5 Patrice Dumas 2006-09-28 15:40:22 EDT
I didn't asked to have a shorter description but a shorter summary...

For the %description I just asked to have it wrapped at 80 columns.
You have to keep the dot at the end of the description.
Comment 6 Patrice Dumas 2006-09-28 18:14:09 EDT
Some comments for conky are relevant here (NEWS empty, bad
changelog entries).
Comment 7 Michael Rice 2006-10-04 14:20:33 EDT
Ok I have fixed all the warnings and errors from rpmlint, the changelog entries
are now well formatted and detailed.

http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl/2/wmctrl-1.07-2.fc5.src.rpm
http://errr.fluxbox-wiki.org/fedora_stuff/wmctrl/2/wmctrl.spec

Any other suggestions on this package?
Comment 8 Patrice Dumas 2006-10-04 15:46:57 EDT
It seems approvable for me. I have a comment, though which isn't
a blocker. You used wildcards in %files, it is fine and sometimes
unavoidable, but I myself prefer listing a bit more explicitely, 
to notice when something change in the package. It is a matter
of personal preferences, but I would have chosed, in your case, 
something along:

%{_bindir}/wmctrl
%{_mandir}/man1/wmctrl.1*

Now you have to find a sponsor. This package is a bit simple
but you shown you were able to follow the guidelines.
I am ready to sponsor you if conky is accepted, but since it is
a much more complicated package, I'll also sponsor you if you
do usefull comments on other reviews. That's the first time I
am in a position to sponsor somebody so I hope I'll be good
sponsor ;-)
Comment 9 Patrice Dumas 2006-10-04 16:33:59 EDT
Another remark, the timestamp of the sources is bad. To have
the right timestmap, you can use wget -N on the url, or 
spectool -g on the specfile. It is better to have the right 
timestamps, it gives some usefull information.
Comment 10 Jason Tibbitts 2006-10-04 18:31:38 EDT
Patrice, why don't you just sponsor him?
Comment 11 Jason Tibbitts 2006-10-04 18:33:15 EDT
Umm, duh, please just ignore me.
Comment 12 Michael Rice 2006-10-04 20:37:49 EDT
Wow this is great, thanks. I will look into this timestamp issue. As for conky I
am some what stuck on it right now because I can get it to build and run fine if
I build it from ~/ but if I build with mock it keeps dumping a core when its
started... Im not sure how to trouble shoot it further than what I have it now.
Comment 13 Paul Howarth 2006-10-05 03:53:42 EDT
(In reply to comment #12)
> Wow this is great, thanks. I will look into this timestamp issue. As for conky I
> am some what stuck on it right now because I can get it to build and run fine if
> I build it from ~/ but if I build with mock it keeps dumping a core when its
> started... Im not sure how to trouble shoot it further than what I have it now.

Try using rpmdiff (from the rpmlint package) to see what the difference is
between your locally-built package and one built in mock. That might give you a
pointer.
Comment 14 Patrice Dumas 2006-10-05 09:29:14 EDT
* rpmlint is silent
* follow packaging/naming guidelines
* spec legible
* match upstream
1fe3c7a2caa6071e071ba34f587e1555  wmctrl-1.07.tar.gz
* sane provides
* right buildrequires
* free software, licence included

I'll sponsor you.

APPROVED

Note You need to log in before you can comment on or make changes to this bug.