Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1163440 (plasma-oxygen) - Review Request: plasma-oxygen - KDE and Qt widget style and window decorations
Summary: Review Request: plasma-oxygen - KDE and Qt widget style and window decorations
Keywords:
Status: CLOSED RAWHIDE
Alias: plasma-oxygen
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: kwin
Blocks: kde-reviews plasma5
TreeView+ depends on / blocked
 
Reported: 2014-11-12 17:13 UTC by Daniel Vrátil
Modified: 2015-11-02 01:38 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-01-20 13:40:45 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Daniel Vrátil 2014-11-12 17:13:49 UTC
Spec URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygens.spec
SRPM URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen-5.1.1-2.fc20.src.rpm
Description: Plasma and Qt widget style and window decorations for Plasma 5 and KDE 4
Fedora Account System Username: dvratil

Comment 1 Kevin Kofler 2014-11-12 17:38:17 UTC
A quick glance at the specfile (after fixing the typo in the URL):
1. I would version the "Obsoletes:      oxygen-plasma-kde4".
2. Typo in oxygen-cursor-themes: "Obsoletes:      plasma-oxygen-common < 5.1.1.-2" – stray '.' before '-'.

Comment 2 Kevin Kofler 2014-11-12 22:12:15 UTC
Actually, as found by Dan Mossor on IRC, "Obsoletes:      oxygen-plasma-kde4" is entirely wrong, should be "Obsoletes:      plasma-oxygen-kde4 < some version" (the name is also wrong).

Comment 3 Kevin Kofler 2014-11-14 11:08:44 UTC
And as said on IRC (another upgrade path issue reported by Dan Mossor), the main (meta)package is missing, please add an empty "%files" list. (No %files list means no main package. An empty %files list means an empty main package. That's not the same.)

And please post updated "Spec URL" (s/oxygens/oxygen/) and especially "SRPM URL" links here.

Comment 4 Daniel Vrátil 2014-11-14 13:36:16 UTC
Spec URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen.spec
SRPM URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen-5.1.1-6.fc20.src.rpm

Thanks for the comments.

- fixed the Obsoletes/Conflicts issue (hopefully)
- created empty %files section for main package

Comment 5 Kevin Kofler 2014-11-14 16:23:17 UTC
The Obsoletes are still wrong. Why "Obsoletes:      plasma-oxygen-kde4 < 5.1.1-1" in oxygen-cursor-themes and oxygen-sound-theme? This should be in qt4-style-oxygen instead of (or in addition to if you had shipped both names at some point in the past) your "Obsoletes:      oxygen-plasma-kde4". oxygen-cursor-themes and oxygen-sound-theme should have "Obsoletes:      plasma-oxygen-common < 5.1.1-1" instead, as they had in the first version of the specfile (but without the "5.1.1." typo ;-) ).

Comment 6 Kevin Kofler 2014-11-14 16:37:52 UTC
And you actually need to "Obsoletes:      plasma-oxygen-kde4 < 5.1.1-2", because you shipped a plasma-oxygen-kde4-5.1.1-1%{?dist}.

Comment 8 Rex Dieter 2014-11-18 17:48:20 UTC
naming: ok
lots of subpkgs going on there, but it looks pretty good, upgrade paths are obviously tricky, but I think it's mostly sorted out now.

1. SHOULD remove from kwin-oxygen:
# Conflicts with kde-style-oxygen from kde-workspace
Conflicts:      kde-style-oxygen < 5.1.0-1
pretty sure this isn't true, and can be removed

2. SHOULD omit from qt4-style-oxygen and qt5-style-oxygen:
Requires: oxygen-cursor-themes = %{version}-%{release}
Requires: oxygen-sound-theme = %{version}-%{release}
I don't think these are needed, the widget style is/(should-be?) independent of cursor-theme and sounds

sources: ok
fc112c113dedcbac338e0db89af4b388  oxygen-5.1.1.tar.xz

macros: ok, though preferably
3. SHOULD use
make install/fast DESTDIR=%{buildroot}
rather than
%make_install

scriptlets: NOT OK
4. MUST add iconcache scriptlets to oxygen-cursor-themes subpkg

license: ok , but I think from licensecheck output we could bump to GPLv2+ if you want

Comment 9 Daniel Vrátil 2014-11-19 13:38:55 UTC
Spec URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen.spec
SRPM URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen-5.1.1-8.fc20.src.rpm

- Remove Conflicts kde-style-oxygen from kwin-oxygen
- Remove Requires themes from qt{4,5}-style-oxygen
- Fixed scriptlets

Comment 10 Rex Dieter 2014-11-19 13:41:24 UTC
Looks good, thanks, APPROVED

Comment 11 Daniel Vrátil 2014-11-19 13:58:46 UTC
New Package SCM Request
=======================
Package Name: plasma-oxygen
Short Description: Plasma and Qt widget style and window decorations for PLasma 5 and KDE 4
Upstream URL: https://projects.kde.org/projects/kde/workspace/oxygen
Owners: dvratil ltinkl jgrulich kkofler rdieter than
Branches: f20 f21
InitialCC:

Comment 12 Gwyn Ciesla 2014-11-19 14:08:10 UTC
Git done (by process-git-requests).

Comment 13 Rex Dieter 2014-12-03 19:18:34 UTC
I'll reset to modified since there aren't any (rawhide) builds yet


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