Bug 851890 (mate-window-manager) - Review Request: mate-window-manager - MATE Desktop window manager
Summary: Review Request: mate-window-manager - MATE Desktop window manager
Keywords:
Status: CLOSED ERRATA
Alias: mate-window-manager
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: MATE-DE-tracker
TreeView+ depends on / blocked
 
Reported: 2012-08-26 23:32 UTC by Dan Mashal
Modified: 2012-09-17 22:49 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-09-03 00:47:20 UTC
Type: Bug
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Comment 1 Rex Dieter 2012-08-26 23:47:15 UTC
MUST add mateconf schema scriptlets

SHOULD remove  needless ldconfig -devel scriptlet

SHOULD remove from  -devel
Requires: %{name}%{?_isa} = %{version}-%{release}
(the dep on -libs is enough)

Comment 3 Rex Dieter 2012-08-27 00:27:20 UTC
naming: ok

sources: ok 
b111a2ea36b6fea4603d63e4dc252092  mate-window-manager-1.4.0.tar.xz

licensing: NOT ok
1. according to licensecheck, all sources are 
GPL (v2 or later) (with incorrect FSF address)
so MUST use
License:  GPLv2+

scriptlets: ok

2.  -devel  SHOULD  drop
Requires:  %{name}%{?_isa} = %{version}-%{release}
depending on -libs is enough

3.  main pkg  MUST
Requires: %{name}-libs%{?_isa} = %{version}-%{release}

macros: not ok
4. MUST not use undefined %{po_package} macro, either define it, or just use %{name} in place of it

5.  MUST own theme parent dirs, instead of:
%{_datadir}/themes/ClearlooksRe/metacity-1/
%{_datadir}/themes/Dopple-Left/metacity-1/
%{_datadir}/themes/Dopple/metacity-1/
%{_datadir}/themes/DustBlue/metacity-1/
%{_datadir}/themes/Spidey-Left/metacity-1
%{_datadir}/themes/Spidey/metacity-1/
%{_datadir}/themes/Splint-Left/metacity-1/
%{_datadir}/themes/Splint/metacity-1/
%{_datadir}/themes/WinMe/metacity-1/
%{_datadir}/themes/eOS/metacity-1/
use
%{_datadir}/themes/ClearlooksRe/
%{_datadir}/themes/Dopple-Left/
%{_datadir}/themes/Dopple/
%{_datadir}/themes/DustBlue/
%{_datadir}/themes/Spidey-Left/
%{_datadir}/themes/Spidey/
%{_datadir}/themes/Splint-Left/
%{_datadir}/themes/Splint/
%{_datadir}/themes/WinMe/
%{_datadir}/themes/eOS/
or even just
%{_datadir}/themes/*

Comment 5 Rex Dieter 2012-08-27 01:18:41 UTC
missing
Requires(pre):   mate-conf
Requires(post):  mate-conf
Requires(preun): mate-conf

please add prior to doing any builds.


APPROVED.

Comment 6 Dan Mashal 2012-08-27 01:20:09 UTC
added to spec and uploaded.

Comment 7 Dan Mashal 2012-08-27 01:23:04 UTC
New Package SCM Request
=======================
Package Name: mate-window-manger
Short Description: MATE Desktop window manager
Owners: vicodan rdieter
Branches: f16 f17 f18

Comment 8 Gwyn Ciesla 2012-08-27 01:52:14 UTC
Git done (by process-git-requests).

Corrected name.

Comment 9 Fedora Update System 2012-08-27 04:01:06 UTC
mate-window-manager-1.4.0-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-window-manager-1.4.0-3.fc18

Comment 10 Fedora Update System 2012-08-27 04:01:18 UTC
mate-window-manager-1.4.0-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-window-manager-1.4.0-3.fc17

Comment 11 Michael S. 2012-08-27 10:44:14 UTC
Like the other mate package, this review missed importants bits :

- desktop file usage X-mate seems incorrect, see #847604 

- the license should also be more explicit, see #847604 and #847419
( especially since this would have shown there is gtk code bundled in the tarball, it seems since a few years ).

Comment 12 Rex Dieter 2012-08-27 12:35:31 UTC
%changelog
* Mon Aug 27 2012 Rex Dieter <rdieter>  1.4.0-4
- main pkg Requires: %%name-libs
- drop needless icon scriptlets
- s|MATE|X-MATE| .desktop Categories on < f18 only
- License: GPLv2+

though your comment about bundled code is ominous,  details?

Comment 13 Michael S. 2012-08-27 13:24:40 UTC
There is code coming from gtk, src/ui/metaaccellabel.c 
but that's a upstream issue. I do not know how much did the code evolved, and if that matter. I guess this should be discussed with gtk and mate upstream ?

Comment 14 Rex Dieter 2012-08-27 13:58:00 UTC
It does say at the top of that file:
/* Marco hacked-up GtkAccelLabel */
which certainly implies that it was done on purpose (for some reason).

Comment 15 Fedora Update System 2012-08-27 16:31:49 UTC
mate-window-manager-1.4.0-3.fc18 has been pushed to the Fedora 18 testing repository.

Comment 16 Fedora Update System 2012-09-03 00:47:20 UTC
mate-window-manager-1.4.0-3.fc17 has been pushed to the Fedora 17 stable repository.

Comment 17 Fedora Update System 2012-09-03 08:07:52 UTC
mate-window-manager-1.4.1-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-window-manager-1.4.1-1.fc18

Comment 18 Fedora Update System 2012-09-03 08:08:03 UTC
mate-window-manager-1.4.1-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-window-manager-1.4.1-1.fc17

Comment 19 Fedora Update System 2012-09-17 22:49:15 UTC
mate-window-manager-1.4.0-3.fc18 has been pushed to the Fedora 18 stable repository.


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