Bug 1161637 - Review Request: gnome-shell-extension-background-logo - Background logo extension for GNOME Shell
Summary: Review Request: gnome-shell-extension-background-logo - Background logo exten...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: AcceptedFreezeException
Depends On:
Blocks: F21FinalFreezeException
TreeView+ depends on / blocked
 
Reported: 2014-11-07 14:30 UTC by Kalev Lember
Modified: 2014-11-25 03:05 UTC (History)
6 users (show)

Fixed In Version: fedora-repos-21-2
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-11-25 03:05:24 UTC
Type: ---
mclasen: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Kalev Lember 2014-11-07 14:30:29 UTC
Spec URL: https://kalev.fedorapeople.org/gnome-shell-extension-background-logo.spec
SRPM URL: https://kalev.fedorapeople.org/gnome-shell-extension-background-logo-3.14.0-1.fc22.src.rpm
Description: This is a new GNOME Shell extension to superimpose a Fedora Workstation logo on the screen background in Fedora Workstation installations.
Fedora Account System Username: kalev

Comment 1 Matthias Clasen 2014-11-07 16:09:56 UTC
rpmlint output:

$  rpmlint ~/Downloads/gnome-shell-extension-background-logo-3.14.0-1.fc22.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

rpmlint ~/rpmbuild/RPMS/noarch/gnome-shell-extension-background-logo-3.14.0-1.fc21.noarch.rpm 
gnome-shell-extension-background-logo.noarch: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Comment 2 Matthias Clasen 2014-11-07 16:27:21 UTC
Package name: ok. It doesn't strictly derive from the tarball name, 
   but follows the established naming scheme for packaged gnome-shell extensions
Spec file name: ok
Packaging guidelines: Only change I would recommend is to avoid %make_install. 
   See https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
License: ok
License field: ok
License file: ok
spec language: ok
spec legibility: legible
upstream sources: ok
buildable: yes
excludearch: n/a
buildrequires: ok
locales: n/a
ldconfig: n/a
bundling: none
relocatable: no
directory ownership: ok
duplicate files: ok
macro use: ok
large docs: n/a
%doc content: ok
static libs: n/a
-devel subpackage: n/a
-devel requires: n/a
libtool archives: n/a
gui apps: none
duplicate directory ownership: the /usr/share/gnome-shell/extensions directory
    is co-owned with the gnome-shell-extension-common package. Which is ok, 
    but I think we could make it a dependency instead without much harm.
utf8 filenames: ok


Summary: ditch %make_install, and consider depending on gnome-shell-extension-common, then I'll approve

Comment 3 Kalev Lember 2014-11-07 16:37:21 UTC
%make_install is a new macro that's supposed to be doing things right -- it's different from the old %makeinstall. The very last sentence in https://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.25makeinstall_macro_should_not_be_used suggests using the new macro too.

Regarding co-owning the /usr/share/gnome-shell/extensions directory, I am not sure it would be right to depend on gnome-shell-extension-common either -- it does indeed own the directory, but it also ships a bunch of unrelated translation files for the extensions that are built from the gnome-shell-extensions module. Maybe gnome-shell could own that directory instead?

Comment 4 Matthias Clasen 2014-11-07 16:39:11 UTC
Alright, %make_install it is!

> Maybe gnome-shell could own that directory instead?

Yeah, that would seems a little better to me. But we don't have to solve that in this review.

Comment 5 Kalev Lember 2014-11-07 16:46:38 UTC
Thanks Matthias!

New Package SCM Request
=======================
Package Name: gnome-shell-extension-background-logo
Short Description: Background logo extension for GNOME Shell
Upstream URL: https://extensions.gnome.org/extension/889/background-logo/
Owners: fmuellner kalev
Branches: f21
InitialCC:

Comment 6 Gwyn Ciesla 2014-11-07 16:57:38 UTC
Git done (by process-git-requests).

Comment 7 Fedora Blocker Bugs Application 2014-11-18 13:58:43 UTC
Proposed as a Freeze Exception for 21-final by Fedora user sgallagh using the blocker tracking app because:

 Getting the branding right for Final is very important. This change should be minimally-invasive; it just adds a brand image overlayed atop the default wallpaper.

Comment 8 Florian Müllner 2014-11-18 19:13:00 UTC
(In reply to Kalev Lember from comment #5)
> New Package SCM Request
> [...]
> Upstream URL: https://extensions.gnome.org/extension/889/background-logo/

Would https://git.fedorahosted.org/git/background-logo-extension.git be better?

Comment 9 Mike Ruckman 2014-11-19 18:42:41 UTC
Discussed in 2014-11-19 blocker review meeting. This seems low risk and adjusts branding.

Comment 10 Kalev Lember 2014-11-20 16:45:02 UTC
(In reply to Florian Müllner from comment #8)
> (In reply to Kalev Lember from comment #5)
> > New Package SCM Request
> > [...]
> > Upstream URL: https://extensions.gnome.org/extension/889/background-logo/
> 
> Would https://git.fedorahosted.org/git/background-logo-extension.git be
> better?

Thanks, fixed in the spec file I just pushed.

Comment 11 Kalev Lember 2014-11-20 18:32:22 UTC
The other part of this is to enable the extension by default -- I've sent patches to that effect to https://lists.fedoraproject.org/pipermail/rel-eng/2014-November/018968.html

Comment 12 Adam Williamson 2014-11-21 23:28:59 UTC
This appears to have been built but not submitted as an update. It cannot be included in composes until it is submitted to Bodhi.

Comment 13 Kalev Lember 2014-11-21 23:30:54 UTC
I'll submit it together with the fedora-release build once we have it. It's waiting on dgilmore to review the patch.

Comment 14 Fedora Update System 2014-11-22 01:12:56 UTC
fedora-repos-21-2,fedora-release-21-2,gnome-shell-extension-background-logo-3.14.0-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/fedora-repos-21-2,fedora-release-21-2,gnome-shell-extension-background-logo-3.14.0-1.fc21

Comment 15 Fedora Update System 2014-11-22 20:21:02 UTC
Package fedora-repos-21-2, fedora-release-21-2, gnome-shell-extension-background-logo-3.14.0-1.fc21:
* should fix your issue,
* was pushed to the Fedora 21 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing fedora-repos-21-2 fedora-release-21-2 gnome-shell-extension-background-logo-3.14.0-1.fc21'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-15595/fedora-repos-21-2,fedora-release-21-2,gnome-shell-extension-background-logo-3.14.0-1.fc21
then log in and leave karma (feedback).

Comment 16 Fedora Update System 2014-11-25 03:05:24 UTC
fedora-repos-21-2, fedora-release-21-2, gnome-shell-extension-background-logo-3.14.0-1.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.


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