Bug 498125

- Require system-icon-theme instead of fedora-icon-theme, as one should be able to remove Fedora brandings altogether.

- Is the obsoletes&provides really necessary since there AFAIK is not and hasn't ever been a lxde-settings package in Fedora?

rpmlint output is clean.

MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
- lxde-settings-daemon.c is under GPLv2+, other files are under MIT.
- I'm not sure, however, if the MIT license needs to be mentioned since it's compatible with GPLv2+.

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. OK
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. NEEDSFIX
- Package already Requires: lxde-common which provides /usr/share/lxde => remove
 %dir %{_datadir}/lxde/

MUST: Files only listed once in %files listings. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK
MUST: All relevant items are included in %doc. Items in %doc do not affect  runtime of application. OK
MUST: Header files must be in a -devel package. OK
MUST: Static libraries must be in a -static package. OK
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. OK
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. OK
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. OK
MUST: Packages does not contain any .la libtool archives. OK
MUST: Desktop files are installed properly. OK
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

(In reply to comment #1)
> - Require system-icon-theme instead of fedora-icon-theme, as one should be able 
> to remove Fedora brandings altogether.

The reason for choosing fedora-icon-theme over system-icon-theme is that I explicitly set Fedora in the patch. Changed.

> - Is the obsoletes&provides really necessary since there AFAIK is not and
> hasn't ever been a lxde-settings package in Fedora?

You are correct. I made one as a subpackage of the current lxde-common, but I never built it. Removed.

> - lxde-settings-daemon.c is under GPLv2+, other files are under MIT.
> - I'm not sure, however, if the MIT license needs to be mentioned since it's
compatible with GPLv2+.

Agreed and removed. BTW: The included COPYING is GPLv3, not v2. Upstream confirmed this was an error. You want me to replace the file in the package or leave it as is for now.

> - Package already Requires: lxde-common which provides /usr/share/lxde =>
> remove
>  %dir %{_datadir}/lxde/

That was intentional. I omitted the dir from lxde-common-0.4 and made it require lxde-settings-daemon instead.


Updated package:
SPEC: http://cwickert.fedorapeople.org/review/lxde-settings-daemon.spec
SRPM: http://cwickert.fedorapeople.org/review/lxde-settings-daemon-0.4-2.fc11.src.rpm

(In reply to comment #2)
> > - lxde-settings-daemon.c is under GPLv2+, other files are under MIT.
> > - I'm not sure, however, if the MIT license needs to be mentioned since it's
> compatible with GPLv2+.
> 
> Agreed and removed. BTW: The included COPYING is GPLv3, not v2. Upstream
> confirmed this was an error. You want me to replace the file in the package or
> leave it as is for now.

Leave it as it is. The license of the program is the one in the source code.

> > - Package already Requires: lxde-common which provides /usr/share/lxde =>
> > remove
> >  %dir %{_datadir}/lxde/
> 
> That was intentional. I omitted the dir from lxde-common-0.4 and made it
> require lxde-settings-daemon instead.

Oh, okay, no problem then.

**

The package has been

APPROVED.

New Package CVS Request
=======================
Package Name: lxde-settings-daemon
Short Description: XSettings Daemon for LXDE
Owners: cwickert
Branches: F-10 F-11
InitialCC:

cvs done.

lxde-settings-daemon-0.4-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/lxde-settings-daemon-0.4-2.fc11

lxde-settings-daemon-0.4-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/lxde-settings-daemon-0.4-2.fc10

lxde-settings-daemon-0.4-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/lxde-settings-daemon-0.4-3.fc11

lxde-settings-daemon-0.4-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/lxde-settings-daemon-0.4-3.fc10

lxde-settings-daemon-0.4-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: lxde-settings-daemon
New Branches: EL-4 EL-5
Owners: cwickert

cvs done.

lxde-settings-daemon-0.4-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.