Bug 847697 - (mate-backgrounds) Review Request: mate-backgrounds - Backgrounds for MATE Desktop
Review Request: mate-backgrounds - Backgrounds for MATE Desktop
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks: MATE-DE-tracker
  Show dependency treegraph
 
Reported: 2012-08-13 05:53 EDT by Dan Mashal
Modified: 2012-09-17 18:04 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-22 17:06:16 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 1 Rex Dieter 2012-08-18 20:11:10 EDT
I can review
Comment 2 Rex Dieter 2012-08-18 20:20:57 EDT
naming: ok

sources: ok
$ md5sum *.xz
225a2ce7672c2a236c3b7d01da95af5a  mate-backgrounds-1.4.0.tar.xz

licensing

1 SHOULD remove
BuildRequires: mate-conf-devel mate-corba-devel
these aren't needed (builds ok for me without these)

macros: mostly ok, except for:

2. SHOULD not  use undefined macro po_package, change
%find_lang %{po_package} --all-name
...
%files -f %{po_package}.lang

to
%find_lang %{name} --all-name
...
%files -f %{name}.lang

3. SHOULD:  dir /usr/share/pixmaps/backgrounds seems unowned.  mind either owning this here too, or consider if it's possible to move
/usr/share/pixmaps/backgrounds/mate
to 
/usr/share/backgrounds/mate
to be closer to how gnome does things?  (fwiw, kde uses /usr/share/wallpapers)

scriptlets: ok (n/a)


fairly good and simple, APPROVED.

please address items 1-3 prior to doing any builds.
Comment 3 Dan Mashal 2012-08-18 22:12:53 EDT
New Package SCM Request
=======================
Package Name: mate-backgrounds
Short Description: Backgrounds for MATE Desktop
Owners: vicodan raveit65 rdieter
Branches: f16 f17 f18
Comment 4 Wolfgang Ulbrich 2012-08-19 04:51:24 EDT
missing requrires:

Requires: 		desktop-backgrounds-compat

and for f17 if you really want to build for F16/f17
Requires: 		beefy-miracle-backgrounds-single
Requires: 		beefy-miracle-backgrounds-gnome

For f18 you need the new background packages.

Your package install the backgrounds in 
%{_datadir}/pixmaps/backgrounds/mate/

but fedora use in general a own background path
%{_datadir}/backgrounds/mate/

IMO you have to fix this in a patch, i can send you this patch.
Comment 5 Dan Mashal 2012-08-19 05:02:00 EDT
MATE uses its own background. 

I'm not sure that I want to use Fedora's background. We'll discuss offline.
Comment 6 Rex Dieter 2012-08-19 08:50:26 EDT
I too don't  understand the need for the dependency changes in Comment #4, please explain and provide the justification for doing so.

except for %{_datadir}/pixmaps/backgrounds/mate/, which is something I mentioned  in the review, item 3
Comment 7 Wolfgang Ulbrich 2012-08-19 09:29:53 EDT
(In reply to comment #6)
> I too don't  understand the need for the dependency changes in Comment #4,
> please explain and provide the justification for doing so.
> 
> except for %{_datadir}/pixmaps/backgrounds/mate/, which is something I
> mentioned  in the review, item 3

@ Requires: 		desktop-backgrounds-compat

root@mother rave]# rpm -ql desktop-backgrounds-compat
/usr/share/backgrounds/default.png
/usr/share/backgrounds/images/default-16_10.png
/usr/share/backgrounds/images/default-5_4.png
/usr/share/backgrounds/images/default.png

Without this require we have no /usr/share/backgrounds/default.png
I think nobody wants that.
For get this working we need also the other requires, because 'default.png' is a symlink to current fedora bckground image.

for f16

[root@mother rave]# ls -l /usr/share/backgrounds/default.png 
lrwxrwxrwx 1 root root 40  8. Feb 2012  /usr/share/backgrounds/default.png -> verne/default/standard/verne.png
Comment 8 Rex Dieter 2012-08-19 14:47:53 EDT
the handling of desktop-backgrounds-compat would best be handled elsewhere, particularly when/if you offer a default configuration that actually uses these.

For example, for kde, we provide a kde-settings package that does that.
Comment 9 Wolfgang Ulbrich 2012-08-19 15:15:40 EDT
Which background image Mate-Desktop use in the end is handled by a mateconf key from libmate.
I can add those requires in libmate , np
Comment 10 Rex Dieter 2012-08-19 15:23:28 EDT
like I said, my suggestion would be to put branding/configuration defaults elsewhere,  preferably in a separate package.  one advantage in doing so is that it makes it easier for others to customize and change that way,  but I'll leave the ultimate decision and implementation up to you mate maintainers.
Comment 11 Jon Ciesla 2012-08-19 16:00:01 EDT
Git done (by process-git-requests).
Comment 12 Fedora Update System 2012-08-20 03:39:15 EDT
mate-backgrounds-1.4.0-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-backgrounds-1.4.0-1.fc17
Comment 13 Fedora Update System 2012-08-20 03:39:25 EDT
mate-backgrounds-1.4.0-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-backgrounds-1.4.0-1.fc18
Comment 14 Fedora Update System 2012-08-20 03:39:37 EDT
mate-backgrounds-1.4.0-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-backgrounds-1.4.0-1.fc16
Comment 15 Fedora Update System 2012-08-20 15:50:04 EDT
mate-backgrounds-1.4.0-1.fc18 has been pushed to the Fedora 18 testing repository.
Comment 16 Fedora Update System 2012-08-22 17:06:16 EDT
mate-backgrounds-1.4.0-1.fc17 has been pushed to the Fedora 17 stable repository.
Comment 17 Fedora Update System 2012-08-28 19:30:04 EDT
mate-backgrounds-1.4.0-1.fc16 has been pushed to the Fedora 16 stable repository.
Comment 18 Fedora Update System 2012-09-17 18:04:20 EDT
mate-backgrounds-1.4.0-1.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.