Bug 467943 - Review Request: solar-kde-theme - Solar KDE Theme
Review Request: solar-kde-theme - Solar KDE Theme
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: manuel wolfshant
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-10-21 15:32 EDT by Jaroslav Reznik
Modified: 2008-11-03 06:07 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-11-03 06:07:22 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wolfy: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Jaroslav Reznik 2008-10-21 15:32:00 EDT
Spec URL: http://jreznik.fedorapeople.org/kde/f10/kdm-theme/pkg/solar-kde-theme.spec
SRPM URL: http://jreznik.fedorapeople.org/kde/f10/kdm-theme/pkg/solar-kde-theme-0.1.2-1.fc9.src.rpm
Description: KDE theme based on Fedora 10 Solar artwork containing KDM Solar Mania theme, KSlash Solar Comet theme and Solar background.
Comment 1 manuel wolfshant 2008-10-22 11:04:39 EDT
The rpm is in "works for me state" so I'll review it a bit later today (unless someone beats me to it, I've understood we want this package reviewed asap)
Comment 2 Jaroslav Reznik 2008-10-22 11:10:59 EDT

Spec URL: http://jreznik.fedorapeople.org/kde/f10/solar-kde-theme/solar-kde-theme.spec
SRPM URL: http://jreznik.fedorapeople.org/kde/f10/solar-kde-theme/solar-kde-theme-0.1.3-1.fc9.src.rpm

Still missing to package default user face icon.
Comment 3 Rex Dieter 2008-10-22 11:22:58 EDT
scratch build:
Comment 4 Kevin Kofler 2008-10-22 11:52:44 EDT
Just a note: the fedora.png in the KSplash theme will have to be removed from this packages and added to fedora-logos instead. There also has to be a replacement with the same name and path in generic-logos (without the Fedora trademark in it), so maybe a neutral name like branding.png would be better than fedora.png?
Comment 5 Rex Dieter 2008-10-22 12:01:18 EDT
0. URL: http://fedoraproject.org/wiki/Artwork/F10Themes/Solar
1. License: needs adjusting too, but I'm not sure what too (I'll ask folks in #fedora-art)
Comment 6 manuel wolfshant 2008-10-22 12:35:18 EDT
 I notice that the FedoraWaves theme has its own folder under wallpapers so I guess I need a bit of help here: is the location /usr/share/wallpapers/{solar.png,solar.png.desktop} really the intended one?
Comment 7 Rex Dieter 2008-10-22 12:41:07 EDT
wolfy, both styles of wallpaper works, but maybe we should put it into it's own dir too.
Comment 8 Rex Dieter 2008-10-22 13:00:41 EDT
solar-backgrounds has:
License: CC-BY-SA
Comment 9 Kevin Kofler 2008-10-22 15:17:44 EDT
There should be a directory layout like for Fedora Waves, because that way the theme can be set in systemsettings (which expects a layout like that), and it also allows supporting multiple resolutions (we can use the %1 and %2 placeholders in the default plasmarc, e.g. we had wallpaper=Fedora_Waves/contents/images/%1x%2.png).

        Kevin Kofler
Comment 10 manuel wolfshant 2008-10-22 15:57:59 EDT
So, I guess the current situation is as follows
- License should be CC-BY-SA
- directory layout should be made more similar to the other existing ones (-> move /usr/share/wallpapers/{solar.png,solar.png.desktop to its own folder)
- Source0 should either be a real URL or a comment describing why there is none should be provided
- rpmlint has a complaint which seems valid:
   solar-kde-theme.src: W: non-standard-group User Interface/Desktop

We  also have
   solar-kde-theme.noarch: W: no-documentation
but this one is safe to ignore
Comment 11 Kevin Kofler 2008-10-22 16:03:34 EDT
Well, normally there should be at least one %doc file: COPYING (or LICENSE or License.txt or however you fancy calling it ;-) ). Except if upstream doesn't include it, but here we are upstream, so I don't see a good reason not to. :-)
Comment 12 manuel wolfshant 2008-10-22 16:19:14 EDT
I had no complaints because the bundled tsolar-kde-theme-0.1.3.tar.gz does not include any license info. Which , btw, it should do.
And there also should be a cosy home for it, so that we could specify a real Source0, together with the URL from comment #5
Comment 13 Jaroslav Reznik 2008-10-22 17:21:45 EDT
- [4] Maybe something like solar-ksplash-logo.png for fedora.png. 
- [6, 7, 9] I'll prepare layout similar to FedoraWaves for backgrounds. 
- [5, 8, 10] Licence is OK for me (by-sa), we are attributing sstorari in splash and KDM theme, but still there should be package description with attribution for him for background. For source I think mention that we are upstream and content of src.rpm is ustream is enough.
Comment 14 Kevin Kofler 2008-10-22 17:31:28 EDT
The tarball should contain a README with contents like "This is Free artwork, it can be used under the terms of ... See COPYING for details.", attribution of all the authors and maybe a warranty disclaimer, and a COPYING with the actual text of the CC-BY-SA license. Both should be included as %doc in the package.
Comment 15 Kevin Kofler 2008-10-22 17:43:00 EDT
For the boilerplate, something like this should work:

    These themes are free artwork: you can redistribute them and/or modify
    them under the terms of the Creative Commons Attribution-Share Alike
    [what version?] license as published by Creative Commons.

    These themes are distributed in the hope that they will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    Creative Commons Attribution-Share Alike license for more details.

    You should have received a copy of the Creative Commons Attribution-Share
    Alike License along with this program.  If not, see

(Where I wrote "[what version?]", this should say something like "3.0 Unported". The link at the bottom should be made to match.)
Comment 16 Jaroslav Reznik 2008-10-23 06:55:20 EDT
From now Solar artwork is dual licensed CC-BY-SA and GPLv2+. So we should dual license it too. Artwork as CC-BY-SA and code for themes as GPLv2+.
I've just solved bug with repainting and colours for userlist.
Comment 17 Jaroslav Reznik 2008-10-23 09:35:33 EDT
Update for testing purposes only!

Spec URL: http://jreznik.fedorapeople.org/kde/f10/solar-kde-theme/solar-kde-theme.spec
SRPM URL: http://jreznik.fedorapeople.org/kde/f10/solar-kde-theme/solar-kde-theme-0.1.4-1.fc9.src.rpm

- KDM theme (input backgrounds disappear a little later, bug?)
- backgrounds
- licenses

- move fedora logo to fedora-logos, generic logo
- default face icon (how-to link it?)
- shared backgrounds for KDM, KSplash & Plasma
Comment 18 Rex Dieter 2008-10-23 10:57:29 EDT
Scratch build:

Looks real good here.  I don't see any remaining blockers, wolfy?
Comment 19 manuel wolfshant 2008-10-23 11:23:03 EDT
No blockers in a fast-forward review. APPROVED.
Comment 20 Rex Dieter 2008-10-23 11:31:05 EDT
New Package CVS Request
Package Name: solar-kde-theme
Short Description: Solar KDE theme
Owners: jreznik,than,ltinkl,kkofler,rdieter
Branches: F-9
Comment 21 Kevin Kofler 2008-10-23 15:15:42 EDT
> - default face icon (how-to link it?)

That's kdebase-workspace's job. I will update kdebase-workspace.
Comment 22 Kevin Fenzi 2008-10-23 16:31:47 EDT
cvs done.
Comment 23 Jaroslav Reznik 2008-11-03 06:07:22 EST
Package is in f10-final, not intended for f9, closing. Thanks for help!

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