Bug 284961 - Review Request: serenity - KDE Style and Window Decoration
Review Request: serenity - KDE Style and Window Decoration
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chitlesh GOORAH
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-10 14:17 EDT by Thomas Moschny
Modified: 2008-02-07 15:59 EST (History)
5 users (show)

See Also:
Fixed In Version: 1.7.1-3.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-07 15:58:54 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chitlesh: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
pass qt paths to %configure (1.46 KB, patch)
2008-01-19 18:41 EST, Thomas Moschny
no flags Details | Diff

  None (edit)
Description Thomas Moschny 2007-09-10 14:17:34 EDT
Spec URL: http://thm.1erlei.de/serenity.spec
SRPM URL: http://thm.1erlei.de/serenity-1.7.1-1.fc7.src.rpm
Description (from the author's website): Serenity is a soft and quiet theme to ease your mind, with a lot of options to make the Serenity yours.

This is my second package, and I am looking for a sponsor.
Comment 1 Chitlesh GOORAH 2007-09-12 17:28:55 EDT
(In reply to comment #0)
> This is my second package, and I am looking for a sponsor.

If it is your second package, how did you submit your first package ?
what is the name of your first package?

I can sponsor you but first I would like to see whether you have understood 
the fedora packaging guidelines. So please do at least 2 informal reviews of 
any package of your choice by adding me as CC: to that bug.

#001: As for your %docs, copy/rename docs, so we don't need subdirs in docdir, 
I'm ok with it.

#002: BuildRequires:  kdebase-devel >= 3.0
Remove >= 3.0, as fedora core 6 and onwards already has a kde beyond 3.0

#003: # FIXME: ldconfig needed?
no it doesn't. use rpmlint to verify.
Comment 2 Chitlesh GOORAH 2007-09-12 17:32:00 EDT
#004: 
BuildRoot:      %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
Please refer to the fedora packaging guidelines and update the BuildRoot
Comment 3 Thomas Moschny 2007-09-12 18:39:14 EDT
Spec URL: http://thm.1erlei.de/serenity.spec (updated)
SRPM URL: http://thm.1erlei.de/serenity-1.7.1-2.fc7.src.rpm

(In reply to comment #1)
> If it is your second package, how did you submit your first package ?
> what is the name of your first package?

Sorry, this was misleading. The first package, python-textile, also needs to 
be reviewed, see #284101.

> I can sponsor you but first I would like to see whether you have understood 
> the fedora packaging guidelines. So please do at least 2 informal reviews of 
> any package of your choice by adding me as CC: to that bug.

Ok, will do that.

> #002: BuildRequires:  kdebase-devel >= 3.0
> Remove >= 3.0, as fedora core 6 and onwards already has a kde beyond 3.0

Fixed.

> #003: # FIXME: ldconfig needed?
> no it doesn't. use rpmlint to verify.

Removed.

(In reply to comment #2)
> #004: 
> 
BuildRoot:      %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> Please refer to the fedora packaging guidelines and update the BuildRoot
> 

This seems to be a bit controversial, but 
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473
clearly recommends the above line for BuildRoot.
Comment 4 Chitlesh GOORAH 2007-12-30 11:10:49 EST
Please note that this package can only be available for the following branch:
* F-7
* F-8

KDE4 will be in F-9. So, serenity should be ported to KDE4 so that it could be
approved for F-9.
Comment 5 Chitlesh GOORAH 2007-12-30 11:26:31 EST
MUST Items:

- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- 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 spec file for the package is be legible. 
- 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: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package containing GUI applications includes a %{name}.desktop file, 
and that file must be properly installed with desktop-file-install in 
the %install section.
- 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 subpackages present.

APPROVED
Comment 6 Chitlesh GOORAH 2008-01-19 10:29:05 EST
Ping ?

what is the status of this package ?
Comment 7 Thomas Moschny 2008-01-19 18:40:03 EST
In order to build on f8, I had to modify the package, so that qt paths are 
passed to %configure.

To avoid a complete re-review, I'll attach the diff.

Spec URL: http://thm.1erlei.de/serenity.spec
SRPM URL: http://thm.1erlei.de/serenity-1.7.1-3.fc8.src.rpm

Next step would be applying for an account, I guess?

Comment 8 Thomas Moschny 2008-01-19 18:41:31 EST
Created attachment 292280 [details]
pass qt paths to %configure
Comment 9 Chitlesh GOORAH 2008-01-20 03:51:16 EST
(In reply to comment #7)
> 
> Next step would be applying for an account, I guess?
> 

Get an account on Fedora account system and I'll sponsor you.

If you are interested in packaging KDE applications in the future, you are
gladly welcome to join KDE-SIG at the Fedora project.
Comment 10 Jason Tibbitts 2008-01-22 14:49:23 EST
Chitlesh: "thm" has been in the cvsextras queue for a little while now, if you
want to click the proper button there.
Comment 11 Chitlesh GOORAH 2008-01-22 16:51:15 EST
Thomas, you are now sponsored :)

https://admin.fedora.redhat.com/accounts/userbox.cgi?_editme=Edit&username=thm

You are a Member of the following groups: 
* cla_done(user/approved) 
* cla_fedora(user/approved) 
* cvsextras(user/approved) 
* fedorabugs(user/approved)

Don't forget to build your package.
Comment 12 Thomas Moschny 2008-01-22 17:13:17 EST
Thanks Chitlesh, for the review and for sponsoring me!
Comment 13 Thomas Moschny 2008-01-22 17:53:37 EST
New Package CVS Request
=======================
Package Name: serenity
Short Description: KDE Style and Window Decoration
Owners: thm
Branches: F-7 F-8
InitialCC: none
Cvsextras Commits: yes
Comment 14 Kevin Fenzi 2008-01-22 18:36:48 EST
cvs done.
Comment 15 Fedora Update System 2008-01-24 16:46:17 EST
serenity-1.7.1-3.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update serenity'
Comment 16 Fedora Update System 2008-01-24 16:48:52 EST
serenity-1.7.1-3.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update serenity'
Comment 17 Rex Dieter 2008-01-25 09:09:03 EST
It would appear that this is a KDE3 style only, so should be EOL'd/blocked 
from F-9+ (where KDE4 is in use).  At least until when/if serentiy becomes 
ported and available for KDE4 as well.
Comment 18 Thomas Moschny 2008-01-25 09:46:08 EST
(In reply to comment #17)
> It would appear that this is a KDE3 style only, so should be EOL'd/blocked 
> from F-9+ (where KDE4 is in use). 

Agreed. How / by whom is that to be done?

> At least until when/if serentiy becomes ported and available for KDE4 as 
well.

The author told me that he intends to port serenity to KDE4.
Comment 19 Rex Dieter 2008-01-25 10:11:06 EST
> Agreed. How / by whom is that to be done?

pkg maintainer usually, see also:
http://fedoraproject.org/wiki/PackageMaintainers/RetiredPackages
http://fedoraproject.org/wiki/PackageMaintainers/PackageEndOfLife

> The author told me that he intends to port serenity to KDE4.

good news!
Comment 20 Thomas Moschny 2008-01-26 06:28:20 EST
(In reply to comment #19)
> pkg maintainer usually, see also:
> http://fedoraproject.org/wiki/PackageMaintainers/RetiredPackages
> http://fedoraproject.org/wiki/PackageMaintainers/PackageEndOfLife

Can this easily be reverted when there is a KDE4 port, or should a completely 
new package be created then?
Comment 21 Rex Dieter 2008-01-26 08:41:44 EST
> Can this easily be reverted when there is a KDE4 port?

yes.
Comment 22 Fedora Update System 2008-02-07 15:58:51 EST
serenity-1.7.1-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 23 Fedora Update System 2008-02-07 15:59:35 EST
serenity-1.7.1-3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

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