Bug 851890 (mate-window-manager)

Summary: Review Request: mate-window-manager - MATE Desktop window manager
Product: [Fedora] Fedora Reporter: Dan Mashal <dan.mashal>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: misc, notting, package-review, rdieter
Target Milestone: ---Flags: rdieter: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-09-02 20:47:20 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 840149    

Comment 1 Rex Dieter 2012-08-26 19:47:15 EDT
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-26 20:27:20 EDT
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-26 21:18:41 EDT
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-26 21:20:09 EDT
added to spec and uploaded.
Comment 7 Dan Mashal 2012-08-26 21:23:04 EDT
New Package SCM Request
=======================
Package Name: mate-window-manger
Short Description: MATE Desktop window manager
Owners: vicodan rdieter
Branches: f16 f17 f18
Comment 8 Jon Ciesla 2012-08-26 21:52:14 EDT
Git done (by process-git-requests).

Corrected name.
Comment 9 Fedora Update System 2012-08-27 00:01:06 EDT
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 00:01:18 EDT
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 Scherer 2012-08-27 06:44:14 EDT
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 08:35:31 EDT
%changelog
* Mon Aug 27 2012 Rex Dieter <rdieter@fedoraproject.org>  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 Scherer 2012-08-27 09:24:40 EDT
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 09:58:00 EDT
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 12:31:49 EDT
mate-window-manager-1.4.0-3.fc18 has been pushed to the Fedora 18 testing repository.
Comment 16 Fedora Update System 2012-09-02 20:47:20 EDT
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 04:07:52 EDT
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 04:08:03 EDT
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 18:49:15 EDT
mate-window-manager-1.4.0-3.fc18 has been pushed to the Fedora 18 stable repository.