Bug 1489160

Summary: Review Request: f27-backgrounds - Fedora 27 default desktop background
Product: [Fedora] Fedora Reporter: Luya Tshimbalanga <luya>
Component: Package ReviewAssignee: Robert-André Mauchin 🐧 <eclipseo>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: awilliam, eclipseo, luya, ngompa13, package-review
Target Milestone: ---Flags: eclipseo: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-09-17 01:29:34 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1489164    

Description Luya Tshimbalanga 2017-09-06 20:29:35 UTC
Spec URL: https://luya.fedorapeople.org/packages/SPECS/f27-backgrounds.spec
SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/f27-backgrounds-27.0.0-1.fc26.src.rpm
Description: This package contains desktop backgrounds for the Fedora 27 default
theme.  Pulls in themes for GNOME, KDE, Mate, Cinnamon and Xfce desktops.
Fedora Account System Username:luya

Comment 1 Fedora Blocker Bugs Application 2017-09-06 20:40:41 UTC
Proposed as a Freeze Exception for 27-beta by Fedora user luya using the blocker tracking app because:

 Design team recently sent the wallpaper for beta. The package is ready for review and tested on https://koji.fedoraproject.org/koji/taskinfo?taskID=21693254

Comment 2 Adam Williamson 2017-09-06 20:48:25 UTC
I'd rather have a tracker bug that depends on this as the blocker, because just adding the new package is not *all* that needs to happen. https://bugzilla.redhat.com/show_bug.cgi?id=1489164 . Also, this is a blocker, not an FE, because currently F27 images use the F26 backgrounds, and this breaks the release criteria.

Comment 3 Luya Tshimbalanga 2017-09-06 20:51:34 UTC
Thanks for the correction.

Comment 4 Robert-André Mauchin 🐧 2017-09-07 10:50:52 UTC
First, thank you to all members of the design team for their great work.

Regarding this package:

 - The Source0 is returning 404: https://releases.pagure.org/design/f27-backgrounds-27.0.0.tar.xz doesn't seem to exist.

 - Similarly, the URL: returns an empty wiki page https://fedoraproject.org/wiki/F27_Artwork

 - There's a mix of spaces and tabs in the SPEC, please choose one only and stick with it. (I prefer spaces).

 - In the source archive, there are a lot of backup files included:

$find f27-backgrounds -iname "*~" 
f27-backgrounds/default/f26.xml~
f27-backgrounds/default/mate-backgrounds-f27.xml~
f27-backgrounds/default/mate-backgrounds-f27-animated.xml~
f27-backgrounds/default/f27.xml~
f27-backgrounds/default/f26-animated.xml~
f27-backgrounds/default/f27-animated-new.xml~
f27-backgrounds/default/f27-animated.xml~
f27-backgrounds/default/Makefile~
f27-backgrounds/default/f27-metadata.desktop.desktop~
f27-backgrounds/default/gnome-backgrounds-f27-animated.xml~
f27-backgrounds/default/gnome-backgrounds-f27.xml~

I think these file are generated as backup by Gedit, I don't think they should be included in the final archive.

Comment 5 Luya Tshimbalanga 2017-09-08 17:33:54 UTC
Here is the updated spec and srpm files
- Corrected Source0
- Wiki page now exists
- Tab only used
- Removal of "*~" files in sources

Resulted rpmlint:
rpmlint rpmbuild/SRPMS/f27-backgrounds-27.0.0-2.fc26.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Scratch build
https://koji.fedoraproject.org/koji/taskinfo?taskID=21729091

Spec URL: https://luya.fedorapeople.org/packages/SPECS/f27-backgrounds.spec
SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/f27-backgrounds-27.0.0-2.fc26.src.rpm

Comment 6 Robert-André Mauchin 🐧 2017-09-09 09:35:28 UTC
The requires should be in the form:

Requires: %{name}-base%{_isa} = %{version}-%{release}

You forgot the %{_isa} part.

I accept the 📦 but please fix this before import.

Comment 7 Neal Gompa 2017-09-09 12:21:31 UTC
> You forgot the %{_isa} part.

No %{_isa} is required for noarch packages. In fact, it should *not* be there.

Comment 8 Luya Tshimbalanga 2017-09-09 19:40:42 UTC
Thanks for the review.

Comment 9 Gwyn Ciesla 2017-09-11 14:12:00 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/f27-backgrounds. You may commit to the branch "f27" in about 10 minutes.

Comment 10 Fedora Update System 2017-09-11 19:41:21 UTC
f27-backgrounds-27.0.0-2.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-fee0766883

Comment 11 Fedora Update System 2017-09-11 19:55:11 UTC
desktop-backgrounds-27.0.0-1.fc27 f27-backgrounds-27.0.0-2.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-fee0766883

Comment 12 Fedora Update System 2017-09-12 19:56:16 UTC
desktop-backgrounds-27.0.0-1.fc27, f27-backgrounds-27.0.0-2.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-fee0766883

Comment 13 Fedora Update System 2017-09-17 01:29:34 UTC
desktop-backgrounds-27.0.0-1.fc27, f27-backgrounds-27.0.0-2.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.