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)
Summary: | Review Request: plasma-oxygen - KDE and Qt widget style and window decorations | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Daniel Vrátil <dvratil> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dvratil, jeischma, jgrulich, kevin, package-review, rdieter |
Target Milestone: | --- | Keywords: | Reopened |
Target Release: | --- | Flags: | rdieter:
fedora-review+
gwync: fedora-cvs+ |
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-01-20 13:40:45 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: | 1135522 | ||
Bug Blocks: | 656997, 1135103 |
Description
Daniel Vrátil
2014-11-12 17:13:49 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 '-'. 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). 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. 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 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 ;-) ). 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}. Spec URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen.spec SRPM URL: https://dvratil.fedorapeople.org/plasma5/review/plasma-oxygen-5.1.1-7.fc20.src.rpm 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 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 Looks good, thanks, APPROVED 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: Git done (by process-git-requests). I'll reset to modified since there aren't any (rawhide) builds yet |