Bug 477990 - Review Request: xfce4-settings - Settings Manager for Xfce
Summary: Review Request: xfce4-settings - Settings Manager for Xfce
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-12-27 00:34 UTC by Kevin Fenzi
Modified: 2009-04-14 18:57 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-14 18:57:57 UTC
Type: ---
Embargoed:
lkundrak: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Kevin Fenzi 2008-12-27 00:34:25 UTC
Spec URL: http://www.scrye.com/~kevin/fedora/xfce4-settings/xfce4-settings.spec
SRPM URL: http://www.scrye.com/~kevin/fedora/xfce4-settings/xfce4-settings-4.5.92-1.fc11.src.rpm
Description: 

This package includes the settings manager applications for the Xfce desktop.

Note: this package is part of the upcoming Xfce 4.6. 
You will need: 
libxfce4util-4.5.92-1.fc11.src.rpm
libxfcegui4-4.5.92-1.fc11.src.rpm
libxfce4menu-4.5.92-1.fc11.src.rpm
exo-0.3.92-1.fc11.src.rpm
(In that order)

Comment 1 Lubomir Rintel 2008-12-27 14:04:35 UTC
(In reply to comment #0)
> Note: this package is part of the upcoming Xfce 4.6. 
> You will need:

Apart from the packages you enumerated that were found in xfce4-settings [1] directory, I've also grabbed xfconf [2], since a couple of packages demanded it.

[1] http://www.scrye.com/~kevin/fedora/xfce4-settings/
[2] http://www.scrye.com/~kevin/fedora/xfconf/

* RPMlint is silent and happy
* Spec file is mostly sane and legible
* Compiler flags are used appropriately
* Builds find in mock, with additional repository of xfce packages
* Requires, provides and filelists are sane
* License is correct

A few notes:

1.) Please don't do --vendor ""; in case upstream specifies vendor field, do not reset it, otherwise set it to "fedora"
https://fedoraproject.org/wiki/Packaging/Guidelines#.desktop_file_creation

2.) Please ensure the following is correct:
Build Configuration:
* Installation prefix:       /usr
* Debug Support:             minimum
* Libnotify support:         no
* Xcursor support:           yes
* Sounds settings support    no
* Libxklavier support:       no
Did you really mean to build w/o support for the above?

3.) The following line seems useless:
find $RPM_BUILD_ROOT -name '*.desktop' -ls
I guest it was forgotten there. Please remove it.

I don't think any of the above would warrant a review blocker:

APPROVED

Comment 2 Kevin Fenzi 2008-12-28 01:47:37 UTC
>1.) Please don't do --vendor ""; in case upstream specifies vendor field, do
>not reset it, otherwise set it to "fedora"
>https://fedoraproject.org/wiki/Packaging/Guidelines#.desktop_file_creation

Good catch. Fixed to use fedora as vendor (upstream doesn't have one by default)

>2.) Please ensure the following is correct:
>Build Configuration:
>* Installation prefix:       /usr
>* Debug Support:             minimum
>* Libnotify support:         no
>* Xcursor support:           yes
>* Sounds settings support    no
>* Libxklavier support:       no
>Did you really mean to build w/o support for the above?

Nope. Thanks for spotting those. Added some buildrequires to pick them up. 

>3.) The following line seems useless:
>find $RPM_BUILD_ROOT -name '*.desktop' -ls
>I guest it was forgotten there. Please remove it.

Well, it's needed because the make install installs the desktop files, 
and I then install them again with desktop-file-install. I can just use 
--delete-original instead. I will switch to that. 

>I don't think any of the above would warrant a review blocker:
>
>APPROVED

Thanks!

FYI, new package: 

Spec URL: http://www.scrye.com/~kevin/fedora/xfce4-settings/xfce4-settings.spec
SRPM URL:
http://www.scrye.com/~kevin/fedora/xfce4-settings/xfce4-settings-4.5.92-2.fc11.src.rpm

CVS request: 

New Package CVS Request
=======================
Package Name: xfce4-settings
Short Description: Settings Manager for Xfce
Owners: kevin
Branches: devel
InitialCC:

Comment 3 Kevin Fenzi 2008-12-28 19:05:27 UTC
cvs done.

Comment 4 Lubomir Rintel 2009-04-14 18:57:57 UTC
I think this was imported and built.


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