[Bug 441637] New: Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/show_bug.cgi?id=441637

Summary:  Review Request: wave-kdm-theme
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: low
          Priority: low
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail],[e-mail]

Spec URL: http://than.fedorapeople.org/wave-kdm-theme/wave-kdm-theme.spec
SRPM URL: 
http://than.fedorapeople.org/wave-kdm-theme/wave-kdm-theme-1.0.fc9.src.rpm
Description: 
This package contains the Fedora Wave KDM theme that is the default
theme in Fedora 9.

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary| Review Request: wave-kdm-  |Review Request: wave-kdm-
                   |theme                       |theme

------- Additional Comments From [e-mail]  2008-04-09 05:16 EST -------
http://than.fedorapeople.org/wave-kdm-theme/wave-kdm-theme-1.0-1.fc9.src.rpm

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]    |[e-mail]

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]      |[e-mail]

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]    |[e-mail]

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 06:03 EST -------
Created an attachment (id=301768)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=301768&action=view)
Screenshot with different users in  1280x1024

1. Shouldn't it be "waves" instead of "wave"?
http://fedoraproject.org/wiki/Artwork/F9Themes/Waves

2. For infinity and flyinghigh we've used the prefix "fedora":
fedoraflyinghigh-kdm-theme and fedorainfinity-kdm-theme

So I would suggest "fedorawaves-kdm-theme" as name.

3. Please have a look at the screenshot. It'll need some work:
 - hostname is bigger than the box
 - font color needs some changes for not selected users

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 07:11 EST -------
Sebastian, what color scheme are you using? The author says you should use 
ObsidianBlue.

For the "hostname is bigger than the box" problem, we should just make the box 
wider, there's plenty of room.

I'll do the review when I get back home.

The name should be waves-kdm-theme or fedorawaves-kdm-theme. I'm not sure 
about the "fedora" part because the theme doesn't actually contain a Fedora 
logo, but on the other hand it makes it clear where the theme is coming from.

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 07:30 EST -------
(In reply to comment #3)
> Sebastian, what color scheme are you using? The author says you should use 
> ObsidianBlue.

Oxygen (the default in current kdmrc). Where do I get ObsidianBlue? 
kdebase-workspace only contains ObsidianCoast ATM.
> The name should be waves-kdm-theme or fedorawaves-kdm-theme. I'm not sure 
> about the "fedora" part because the theme doesn't actually contain a Fedora 
> logo, but on the other hand it makes it clear where the theme is coming

from.

AFAIR also fedorainfinity-kdm-theme and fedoraflyinghigh-kdm-theme didn't 
contain a Fedora logo. So I would prefer fedorawaves-kdm-theme as %name 
(and /usr/share/kde4/apps/kdm/themes/FedoraWaves as directory).

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 07:35 EST -------
1. Shouldn't it be "waves" instead of "wave"?
> http://fedoraproject.org/wiki/Artwork/F9Themes/Waves

it's typo, it should be waves.
> For infinity and flyinghigh we've used the prefix "fedora":
>
> fedoraflyinghigh-kdm-theme and fedorainfinity-kdm-theme

i will rename it to fedorawaves-kdm-theme.
> Please have a look at the screenshot. It'll need some work:
> hostname is bigger than the box

it's knowned issue. I already talked with Pavel, he will fix it properly today 
evening. Temporary i will make the box wider.
> font color needs some changes for not selected users

ObsidianCoast works fine for me.

[Bug 441637] Review Request: wave-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: wave-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 07:41 EST -------
ObsidianCoast must be what I meant, that was from memory and I just didn't 
remember it correctly.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: wave-kdm-   |Review Request: fedorawaves-
                   |theme                       |kdm-theme

------- Additional Comments From [e-mail]  2008-04-09 07:43 EST -------
I'm changing the title for the fixed name, please rename the specfile.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 07:48 EST -------
Created an attachment (id=301783)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=301783&action=view)
Screenshot with ColorScheme=ObsidianCoast in 1280x1024

(In reply to comment #6)
> ObsidianCoast must be what I meant, that was from memory and I just didn't 
> remember it correctly.

Ok. :)
ObsidianCoast looks better then.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 08:05 EST -------
BTW: Where does the wallpaper come from? AFAIR the final wallpaper for Waves 
isn't finished yet: 
https://www.redhat.com/archives/fedora-art-list/2008-April/msg00149.html

IMHO it should match the default wallpaper we'll use after a login.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 09:07 EST -------
it's fixed now. could you please review it again? thanks

it's not the final wallpaper. We will update package if there's final 
wallpaper done.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 09:57 EST -------
Spec URL: 
http://than.fedorapeople.org/wave-kdm-theme/fedorawaves-kdm-theme.spec
SRPM URL: 
http://than.fedorapeople.org/wave-kdm-theme/fedorawaves-kdm-theme-1.0-1.f9.s...

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-review?

------- Additional Comments From [e-mail]  2008-04-09 11:19 EST -------
rpmlint has one complaint:
fedorawaves-kdm-theme.noarch: W: no-documentation
Shouldn't there be a COPYING file included?

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 11:22 EST -------
we should ignore this, it could be done later.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

------- Additional Comments From [e-mail]  2008-04-09 11:46 EST -------
MUST Items:
+ rpmlint output:
fedorawaves-kdm-theme.noarch: W: no-documentation
harmless, ignoring
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
+ Packaging Guidelines:
  + License GPL+ OK, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, BuildRequires (none needed), Summary, 
Description
  ! Requires: kde-settings-kdm should be Requires: kdebase-kdm
  + no non-UTF-8 characters
  + no documentation relevant to include
  + nothing to compile, so RPM_OPT_FLAGS are irrelevant
  + nothing to compile (noarch package), so no debuginfo package
  + no static libraries nor .la files
  + no duplicated system libraries
  + no binaries => no rpaths
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + no GUI executables, so no .desktop file needed
  + ... and thus no desktop-file-install needed either (KdmGreeterTheme.desktop 
is not really a .desktop file, it's a theme description)
  + no timestamp-clobbering file commands (cp -p is being used)
  + nothing to build, so _smp_mflags are irrelevant
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
+ no license in the tarball to include as %doc
+ no upstream, skipping source matches upstream check
+ builds on at least one arch (F8 i386)
+ no known non-working arches, so no ExcludeArch needed
+ no BuildRequires needed
+ no translations, so translation/locale guidelines don't apply
+ no shared libraries, so no ldconfig call needed
+ package not relocatable
+ ownership correct (owns package-specific directories, doesn't own directories 
owned by another package)
+ no duplicate files in %files
+ permissions correct, defattr used correctly
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ no large documentation files, so no -doc package needed
+ no %doc files, so no %doc files required at runtime
+ no header files which would need to be in a -devel subpackage
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ no .so symlinks vs. plugins
+ no -devel package, so "-devel should require %{name} = %{version}-%{release}" 
is irrelevant
+ no .la files
+ no GUI executables, so no .desktop file needed
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items:
! no license file in the tarball
+ no translations for description and summary provided by upstream
* skipping mock test (nothing to build, no BRs needed)
* skipping all arch test (it's noarch anyway)
* skipping functionality test (already tested, see previous comments)
+ scriptlets are sane
+ no subpackages other than -devel, so "Usually, subpackages other than devel 
should require the base package using a fully versioned dependency." is 
irrelevant
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies

APPROVED, but please change Requires: kde-settings-kdm to Requires: kdebase-kdm 
after import.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 13:03 EST -------
it's fixed, could you please review it again? thanks

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

------- Additional Comments From [e-mail]  2008-04-09 13:07 EST -------
OK, that resolves the only real issue I found. Just to confirm:
APPROVED

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

------- Additional Comments From [e-mail]  2008-04-09 13:14 EST -------
New Package CVS Request
=======================
Package Name: fedorawaves-kdm-theme
Short Description: Fedora Waves KDM Theme
Owners: than,rdieter,kkofler,ltinkl
Branches: 
InitialCC: 
Cvsextras Commits: yes

For the branches, we need devel only, F-9 can wait for the mass branching.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
               Flag|fedora-cvs?                 |fedora-cvs+

------- Additional Comments From [e-mail]  2008-04-09 22:51 EST -------
cvs done.

[Bug 441637] Review Request: fedorawaves-kdm-theme

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: fedorawaves-kdm-theme

https://bugzilla.redhat.com/show_bug.cgi?id=441637

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |RAWHIDE

------- Additional Comments From [e-mail]  2008-04-10 12:12 EST -------
built in rawhide