Bug 227241 (kde-settings) - Review Request: kde-settings - KDE default configuration
Summary: Review Request: kde-settings - KDE default configuration
Keywords:
Status: CLOSED NEXTRELEASE
Alias: kde-settings
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-02-03 22:05 UTC by Rex Dieter
Modified: 2007-11-30 22:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-05-16 18:31:24 UTC
Type: ---
Embargoed:
chris.stone: fedora-review+
jwboyer: fedora-cvs+


Attachments (Terms of Use)

Description Rex Dieter 2007-02-03 22:05:19 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kde-settings.spec
SRPM URL: http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kde-settings-3.5-17.src.rpm

Put kde config files into separate pkg, so that changing app defaults/settings is easy and modular.

Comment 1 Rex Dieter 2007-02-07 13:36:19 UTC
Will be updating soon, to use kiosktool-style prefs (like how kubuntu works).

Comment 2 Chitlesh GOORAH 2007-03-10 16:31:04 UTC
Can you post the updated srpm for review?

Comment 3 Rex Dieter 2007-03-13 16:01:27 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/kde-settings.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/kde-settings-3.5-18.src.rpm

No kiosk-specific stuff yet, but should be sufficient for reviewing now.

Comment 4 Rex Dieter 2007-04-06 12:37:34 UTC
Chitlesh, ping?

Comment 5 Christopher Stone 2007-04-23 19:06:13 UTC
I'll make an attempt at reviewing this right now.

Comment 6 Rex Dieter 2007-04-27 13:05:44 UTC
Chris, here's the promised "make kdelibs use /etc for config files"
patch/issue-bug-tracker, bug #238136

Comment 7 Christopher Stone 2007-04-27 20:51:27 UTC
==== REVIEW CHECKLIST ====
- rpmlint output:
W: kde-settings no-documentation

okay to ignore.

W: kde-settings hidden-file-or-dir /etc/skel/.kde/Autostart/.directory
W: kde-settings non-conffile-in-etc /etc/skel/.kde/Autostart/.directory
W: kde-settings hidden-file-or-dir /etc/skel/.kde
W: kde-settings hidden-file-or-dir /etc/skel/.kde

okay to ignore, bug probably should be filed against rpmlint for these.

W: kde-settings no-url-tag

okay to ignore.

E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/clockappletrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kcmnspluginrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kdeglobals
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kdesktoprc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kdewizardrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kickerrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kmailrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kminipagerappletrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/knotify.eventsrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/knotifyrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/konquerorrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/konsolerc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/korgacrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/ksmserverrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/ksplashrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/ktiprc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/kwinrc
E: kde-settings file-in-usr-marked-as-conffile /usr/share/config/profilerc

X explained in bug #238136, a comment should be added to the spec.

W: kde-settings no-url-tag

okay to ignore.

W: kde-settings strange-permission xdg_env-hack.sh 0755

what is strange about that?!

W: kde-settings setup-not-quiet

OMG this is so easy to fix.

W: kde-settings mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 3)

OMG this is so easy to fix.

W: kde-settings-kdm no-documentation

okay to ignore.

W: kde-settings-kdm dangling-relative-symlink /etc/kde/kdm/Xsetup
../../X11/xdm/Xsetup_0
W: kde-settings-kdm dangling-relative-symlink /etc/kde/kdm/Xservers
../../X11/xdm/Xservers
W: kde-settings-kdm dangling-relative-symlink /etc/kde/kdm/Xwilling
../../X11/xdm/Xwilling
W: kde-settings-kdm dangling-relative-symlink /etc/kde/kdm/Xaccess
../../X11/xdm/Xaccess
W: kde-settings-kdm dangling-relative-symlink /etc/kde/kdm/Xresources
../../X11/xdm/Xresources

These are okay since it Requires xorg-x11-xdm

W: kde-settings-kdm no-url-tag

okay to ignore.

W: kde-settings-kdm obsolete-not-provided kde-config-kdebase

Why is an obsolte not provided??

W: kde-settings-kdm dangerous-command-in-%pre mv
W: kde-settings-kdm dangerous-command-in-%post cp

OMG these are so easy to fix.

- package named according to package naming guidelines
- spec filename matches %{name}
- package meets packaging guidelines
- package is licensed with open source compatible license
- license matches actual license
- no license in %doc because there really is no license
NOTE: These are just config files, no real license associated with them.
- spec written in American english
- spec file legible
X There are no upstream sources to match against yet no comment either
- package successfully compiles and builds on FC-6 x86_64
- all dependencies listed in BR
- no locales
- no shared libraries in dynamic linkers path
- package is not relocatable
- package owns all directories it creates
- directories it does not create are brought in through Requires
- no duplicates in %files
- file permissions are okay
- contains proper %clean
- macro usage consistent
- contains code
- no large documentation
- files in %doc do not affect runtime
- no header files
- no static libraries
- no pkgconfig files
- no libraries with suffix
- no need for devel subpackage
- no libtool archives
- not a GUI application
? package does not own files or directories owned by other packages
- contains proper %install
- filenames are UTF-8

I still have to verify there are no conflicts with kdebase in devel.


==== MUST FIX ====
- Add comment in spec as mentioned here:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e
- Add a comment in spec file mentioning bug #238136

==== SHOULD FIX ====
- Fix rpmlint warnings:
W: kde-settings setup-not-quiet
W: kde-settings mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 3)
W: kde-settings-kdm obsolete-not-provided kde-config-kdebase
W: kde-settings-kdm dangerous-command-in-%pre mv
W: kde-settings-kdm dangerous-command-in-%post cp

Comment 8 Rex Dieter 2007-04-27 20:57:04 UTC
> I still have to verify there are no conflicts with kdebase in devel.

fyi, likely there are conflicts with current(devel) versions of both kdelibs and
kdebase, mainly because we're moving all/most config's from those two *here*.



Comment 9 Rex Dieter 2007-04-30 18:42:55 UTC
Spec URL: http://www.math.unl.edu/~rdieter/fedora/kde-settings.spec

%changelog
* Mon Apr 30 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 3.5-19
- omit xdg_hack (for now, anyway)
- fc7+: Req: redhat-artwork-kde
- reference: kdelibs: use FHS-friendly /etc/kde (vs. /usr/share/config), bug #238136



Comment 10 Christopher Stone 2007-05-01 18:43:59 UTC
The "dangerous-command-in-%pre/%post" warnings are still there, rpmlint -I is
not helpful.  I have no idea on that one, so lets just ignore.

I have a question about these though:
W: kde-settings obsolete-not-provided kde-config
W: kde-settings-kdm obsolete-not-provided kde-config-kdm

Please let me know why those are not provided?

Please also add a comment as outlined here:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e

and for good meausre add -q to %setup and remove tab in line 3.

Thanks.

Comment 11 Rex Dieter 2007-05-01 19:18:03 UTC
> re: dangerous commands
cp/mv are dangerous, period.  We really ought to keep theme in the scriptlets
for config migration from older packages/distro-releases.

> Please let me know why those are not provided?

Because the Provides are not needed?  They are there only to provide an upgrade
path for previous/existing kde-redhat users.  We should be able to drop this
cruft eventually.

Spec URL: http://www.math.unl.edu/~rdieter/fedora/kde-settings.spec

%changelog
* Tue May 01 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 3.5-20
- don't mix tab/spaces
- %%setup -q


Comment 12 Rex Dieter 2007-05-01 19:18:51 UTC
Oops, truncated changelog:

* Tue May 01 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 3.5-20
- don't mix tab/spaces
- %%setup -q
- Source0 URL comment


Comment 13 Christopher Stone 2007-05-01 19:23:32 UTC
Okay, this looks good.  See comment #8 and comment #11 for explanations of
remaining issues.

*** APPROVED ***


Comment 14 Rex Dieter 2007-05-03 16:01:18 UTC
SRPM: http://apt.kde-redhat.org/apt/kde-redhat/SRPMS/kde-settings-3.5-20.fc7.src.rpm

New Package CVS Request
=======================
Package Name: kde-settings
Short Description: KDE default configuration
Owners: rdieter.edu,than
Branches: devel 

Comment 15 Josh Boyer 2007-05-04 01:19:09 UTC
Done.  In koji, rdieter is the owner of the package.  I'm not sure how to add
than yet so Rex will have to do builds until someone figures it out.

Comment 16 Rex Dieter 2007-05-16 18:31:24 UTC
doh, forgot to close this...


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