Bug 1175323 (plasma-systemsettings) - Review Request: plasma-systemsettings - KDE's System Settings application
Summary: Review Request: plasma-systemsettings - KDE's System Settings application
Keywords:
Status: CLOSED RAWHIDE
Alias: plasma-systemsettings
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:
Blocks: plasma5
TreeView+ depends on / blocked
 
Reported: 2014-12-17 13:49 UTC by Jan Grulich
Modified: 2015-01-20 13:42 UTC (History)
4 users (show)

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


Attachments (Terms of Use)

Description Jan Grulich 2014-12-17 13:49:22 UTC
Spec URL: https://jgrulich.fedorapeople.org/plasma5/plasma-systemsettings.spec
SRPM URL: https://jgrulich.fedorapeople.org/plasma5/plasma-systemsettings-5.1.1-1.fc21.src.rpm
Description: KDE's System Settings application
Fedora Account System Username: jgrulich
Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8410657

Comment 1 Rex Dieter 2015-01-04 16:45:09 UTC
naming: ok (mostly)
1. another pkg where I'm not fond of using an artificial plasma- prefix, SHOULD consider removing it.

2. SHOULD prefer/use
make install/fast DESTDIR=%{buildroot} -C %{_target_platform}
over 
%make_install -C %{_target_platform}

3. MUST validate application .desktop file, add
BuildRequires: desktop-file-utils
and
%check
desktop-file-validate %{buildroot}%{_datadir}/applications/kdesystemsettings.desktop
desktop-file-validate %{buildroot}%{_datadir}/applications/systemsettings.desktop

4. SHOULD use %find_lang ... --with-kde ... to automatically pick up HTML handbooks

5. SHOULD use better project url
URL: https://projects.kde.org/projects/kde/workspace/systemsettings

sources: ok
bb382e4c5b97a4ed8c8e116567247a83  systemsettings-5.1.1.tar.xz

license: ok

Comment 2 Jan Grulich 2015-01-05 12:22:44 UTC
1. That's something I'm not sure with, having a package called just systemsettings in Fedora doesn't seem to be a good option.

2-5. Fixed

Spec URL: https://jgrulich.fedorapeople.org/plasma5/plasma-systemsettings.spec
SRPM URL: https://jgrulich.fedorapeople.org/plasma5/plasma-systemsettings-5.1.1-1.fc21.src.rpm
Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=8528049

Comment 3 Rex Dieter 2015-01-05 12:59:33 UTC
looks good, APPROVED

p.s. make sure to update changelog too when updating pkgs for reviews

Comment 4 Jan Grulich 2015-01-05 13:26:26 UTC
New Package SCM Request
=======================
Package Name: plasma-systemsettings
Short Description: KDE's System Settings application
Upstream URL: https://projects.kde.org/projects/kde/workspace/systemsettings
Owners: @kde-sig rdieter kkofler dvratil than jgrulich ltinkl
Branches: f21
InitialCC:

Comment 5 Gwyn Ciesla 2015-01-05 15:10:55 UTC
WARNING: "@kde-sig" is not a valid FAS account.

Comment 6 Jan Grulich 2015-01-05 15:33:28 UTC
New Package SCM Request
=======================
Package Name: plasma-systemsettings
Short Description: KDE's System Settings application
Upstream URL: https://projects.kde.org/projects/kde/workspace/systemsettings
Owners: group::kde-sig rdieter kkofler dvratil than jgrulich ltinkl
Branches: f21
InitialCC:

Comment 7 Gwyn Ciesla 2015-01-05 18:11:32 UTC
WARNING: "group::kde-sig" is not a valid FAS account.

Comment 8 Jan Grulich 2015-01-05 19:44:02 UTC
New Package SCM Request
=======================
Package Name: plasma-systemsettings
Short Description: KDE's System Settings application
Upstream URL: https://projects.kde.org/projects/kde/workspace/systemsettings
Owners: rdieter kkofler dvratil than jgrulich ltinkl
Branches: f21
InitialCC:

Comment 9 Gwyn Ciesla 2015-01-05 21:13:28 UTC
Git done (by process-git-requests).


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