Bug 467943 - Review Request: solar-kde-theme - Solar KDE Theme
Summary: Review Request: solar-kde-theme - Solar KDE Theme
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: manuel wolfshant
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-10-21 19:32 UTC by Jaroslav Reznik
Modified: 2008-11-03 11:07 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-11-03 11:07:22 UTC
Type: ---
Embargoed:
manuel.wolfshant: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Jaroslav Reznik 2008-10-21 19:32:00 UTC
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 15:04:39 UTC
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 15:10:59 UTC
Update

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 15:22:58 UTC
scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=895772

Comment 4 Kevin Kofler 2008-10-22 15:52:44 UTC
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 16:01:18 UTC
Quickies:
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 16:35:18 UTC
 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 16:41:07 UTC
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 17:00:41 UTC
solar-backgrounds has:
License: CC-BY-SA

Comment 9 Kevin Kofler 2008-10-22 19:17:44 UTC
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 19:57:59 UTC
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 20:03:34 UTC
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 20:19:14 UTC
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 21:21:45 UTC
- [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 21:31:28 UTC
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 21:43:00 UTC
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
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    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
    <http://creativecommons.org/licenses/by-sa/3.0/>.

(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 10:55:20 UTC
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 13:35:33 UTC
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

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

TBD: 
- 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 14:57:29 UTC
Scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=897896

Looks real good here.  I don't see any remaining blockers, wolfy?

Comment 19 manuel wolfshant 2008-10-23 15:23:03 UTC
No blockers in a fast-forward review. APPROVED.

Comment 20 Rex Dieter 2008-10-23 15:31:05 UTC
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 19:15:42 UTC
> - 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 20:31:47 UTC
cvs done.

Comment 23 Jaroslav Reznik 2008-11-03 11:07:22 UTC
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.