Bug 229319

Created attachment 148425 [details]
screenshot

First thing (kde thing), dekorator is useless _without_ the
$RPM_BUILD_ROOT%{_libdir}/kde3/*.la

see screenshot

Strange... I actually packaged those at first, then removed them, and everything
was working..? Probably didn't unload the old module in time.
Ah well, reverting, thanks for the catch! :-)

New build:
Spec URL: http://www.snoekie.com/rpm/dekorator.spec
SRPM URL: http://www.snoekie.com/rpm/dekorator-0.3-2.src.rpm

Changes:
- Added required libtool archives again

Francois, you didn't happen to be using kde-redhat's packages? (:  If so, our 
kdelibs/kdebase include experimental patches to remove the necessity of .la 
files for runtime use.

(In reply to comment #4)
> Francois, you didn't happen to be using kde-redhat's packages?
Haha, busted. :-) Indeed this was the case.
Rex and co, thanks a lot for kde-redhat - excellent stuff, that, and *ahem* I
for one can vouch that your .la patches are working ;-).

But thanks for pointing this out; I'll test my KDE packages on a "clean" Fedora
system as well from now on.

#001: You should explicitly require kdebase, since this package is useless
without kwin.
Requires:      kdebase

#002: other themes ??

On kcontrol, if I select dekorator, then Help, I see
****** In the package ******
deKorator comes with:
    * default-theme - Used by default, illustrates some of deKorator's
      features.
    * ugly-theme - It's purpose is to understand the way deKorator paints the
      decorations.
    * template-theme - this theme  is meant to be a template theme for other
themes,this theme features all possible buttons.
    * Bushido-Yellow-theme - shows how masking can be done.

But your package is only shipping 2 themes:
- default-theme and
- template-theme

#003: Below that same page (help), there is a typo "Allways", it should be
Always. Patch as appropriate and inform upstream.

#004: Is there any reason why you are shipping ugly-theme.tar.gz at
/usr/share/doc/dekorator-0.3/themes/ ?

#005 I've also noticed that /usr/share/doc/dekorator-0.3/themes/deKhelp.xhtml is
a duplicate to the "help" page provided by the kcontrol module.

#006: For the very first time, one select and use dekorator, he/she will be
frameless. Users will freak out. Can you do the necessary so that a default
theme might be chosen by default.

(In reply to comment #6)
> #001: You should explicitly require kdebase, since this package is useless
> without kwin.
> Requires:      kdebase
> 

Ok, will do.

> #002: other themes ??
...
> 
>     * Bushido-Yellow-theme - shows how masking can be done.
> 

The Bushido-Yellow-theme from the tarball does not do its masking properly (on
my system at least) - it cuts out the central window titlebar-area, making it
impossible to see the window title. I will see if I can edit the masks to make
it work properly, otherwise, I'll remove the references from the help file (the
other themes also demonstrate masking anyway).
The theme is also available here:
http://www.kde-look.org/content/show.php?content=32163
...but this version is not compatible with dekorator 0.3.

As for the "K-style: Infinity" theme: the version packaged with dekorator is
broken, according to its author:
http://www.kde-look.org/content/show.php?content=35590
I think a separate package for this theme (using the updated source) would be
better.

> 
> #003: Below that same page (help), there is a typo "Allways", it should be
> Always. Patch as appropriate and inform upstream.
> 

Will do.

> #004: Is there any reason why you are shipping ugly-theme.tar.gz at
> /usr/share/doc/dekorator-0.3/themes/ ?
> 

Simply because it's, well, ugly ;-), and obviously designed to be a teaching
example, not an _actual_ theme... Should I rather install this by default? Or
maybe put it in a subpackage? I was wondering about the "template" theme as
well, but at least that one's usable and semi-nice to look at.

> #005 I've also noticed that /usr/share/doc/dekorator-0.3/themes/deKhelp.xhtml is
> a duplicate to the "help" page provided by the kcontrol module.

Very similar, yes. I will remove this.

> 
> #006: For the very first time, one select and use dekorator, he/she will be
> frameless. Users will freak out. Can you do the necessary so that a default
> theme might be chosen by default.

I agree, this is *very* irritating. I will look into this; it shouldn't be too
hard. I have a question, though: do you think I should make it so that the "Set
Theme Path" button's actions are automatically performed when the user selects a
theme from the list (and thus remove the button completely)? Or should the
program only select an initial default theme, and leave the "set theme path"
button as it is? Personally I feel the "set theme path" button is an unnecessary
and unintuitive step and should be removed, but someone else's opinion would be
greatly appreciated.

(In reply to comment #7)

> The Bushido-Yellow-theme from the tarball does not do its masking properly (on
> my system at least) - it cuts out the central window titlebar-area, making it
> impossible to see the window title. I will see if I can edit the masks to make
> it work properly, otherwise, I'll remove the references from the help file (the
> other themes also demonstrate masking anyway).
> The theme is also available here:
> http://www.kde-look.org/content/show.php?content=32163
> ...but this version is not compatible with dekorator 0.3.
> 
> As for the "K-style: Infinity" theme: the version packaged with dekorator is
> broken, according to its author:
> http://www.kde-look.org/content/show.php?content=35590
> I think a separate package for this theme (using the updated source) would be
> better.
> 

Ok, I'm good with the explanations given.
Can you include a "dekorator.fedora" file to the package which explains this
choice. This "dekorator.fedora" file will be included in %doc


> Simply because it's, well, ugly ;-), and obviously designed to be a teaching
> example, not an _actual_ theme... Should I rather install this by default? Or
> maybe put it in a subpackage? I was wondering about the "template" theme as
> well, but at least that one's usable and semi-nice to look at.

If this "ugly" theme isn't broken, I would like it to be installed as a theme.
We should not forget that our users aren't stupid, they might be waiting to see
it included. :)

> I agree, this is *very* irritating. I will look into this; it shouldn't be too
> hard. I have a question, though: do you think I should make it so that the "Set
> Theme Path" button's actions are automatically performed when the user selects
a theme from the list (and thus remove the button completely)? Or should the
> program only select an initial default theme, and leave the "set theme path"
> button as it is? Personally I feel the "set theme path" button is an unnecessary
> and unintuitive step and should be removed, but someone else's opinion would be
> greatly appreciated.

If you can do it, why not.

New build:
Spec URL: http://www.snoekie.com/rpm/dekorator.spec
SRPM URL: http://www.snoekie.com/rpm/dekorator-0.3-3.src.rpm

Changes:
- Added "Requires: kdebase" as this package is useless without kwin
- Install the "ugly" theme (moved it from %doc)
- Created "remove_theme_paths_button" patch to remove the config dialog's "set
theme paths" button
- Created "default_theme" patch to set a first-time default theme
- Fixed the Bushido-Yellow theme's masks to make it usable
- Install the "Bushido-Yellow" theme by default
- Added "dekorator.fedora" to %doc to explain why some extra themes are
modified/omitted
- Created "config_help_tab" patch to make the config dialog use HTML help (and
fix some typos)

Notes:
(In reply to comment #6)
#002:
I modified the masks for the Bushido-Yellow-theme to make it work properly, and
decided to include it. I did, however create the dekorator.fedora file detailing
the reason for this modification and why "K-Style: Infinity" was left out.

#005:
I have decided _not_ to drop the XHTML documentation and rather modified the
application to make use of it; it looks *much* better this way, especially
because of the use of tables instead of dodgy ascii art. Besides, we now have
the added advantage of being able to read the documentation without using
kcontrol. In addition, I added the 1/2 lines that were missing from it (which
were present in the text-only kcontrol module's original docs. This patch is
fairly portable; if the documentation file is not found, the application will
fall back to the original text... and yes, I fixed that typo :-) (#003). I also
fixed a typo on the "about" dialog.

#006:
Done. The engine now loads the "Default-theme" by default (even when the user
clicks on the "Defaults" button), and I've eliminated the "Set theme button"
completely - configuring the engine now feels much more like configuring a
"normal" kwin decoration.

Great Work.
MUST Items:

- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed (GPL) with an open-source compatible license and
meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own file,
then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The sources used to build the package must matches the upstream source,
as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i386.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.
- MUST: If the package does not contain shared library files located in the
dynamic linker's default paths
- MUST: the package is not designed to be relocatable
- MUST: the package owns all directories that it creates.
- MUST: the package does not contain any duplicate files in the %files listing.
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros section
of Packaging Guidelines.
- MUST: The package contains code, or permissable content. This is described in
detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If it
is in %doc, the program must run properly if it is not present.
- MUST: The package does not contain library files with a suffix 
- MUST: Package does not own files or directories already owned by other packages. 

SHOULD Items:

 - SHOULD: The source package does include license text(s) as COPYING
 - SHOULD: mock builds succcessfully in i386.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD: No scriptlets were used, those scriptlets must be sane. 
 - SHOULD: No subpackages present.

APPROVED!

Follow http://fedoraproject.org/wiki/CVSAdminProcedure for the CVS Request
procedure.

Thanks for the review!

New Package CVS Request
=======================
Package Name: dekorator
Short Description: KDE window decoration engine
Owners: faucamp.za
Branches: FC-5 FC-6
InitialCC: