Bug 284961 - Review Request: serenity - KDE Style and Window Decoration
Summary: Review Request: serenity - KDE Style and Window Decoration
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chitlesh GOORAH
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-09-10 18:17 UTC by Thomas Moschny
Modified: 2008-02-07 20:59 UTC (History)
5 users (show)

Fixed In Version: 1.7.1-3.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-07 20:58:54 UTC
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 23:41 UTC, Thomas Moschny
no flags Details | Diff

Description Thomas Moschny 2007-09-10 18:17:34 UTC
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 21:28:55 UTC
(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 21:32:00 UTC
#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 22:39:14 UTC
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 16:10:49 UTC
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 16:26:31 UTC
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 15:29:05 UTC
Ping ?

what is the status of this package ?

Comment 7 Thomas Moschny 2008-01-19 23:40:03 UTC
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 23:41:31 UTC
Created attachment 292280 [details]
pass qt paths to %configure

Comment 9 Chitlesh GOORAH 2008-01-20 08:51:16 UTC
(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 19:49:23 UTC
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 21:51:15 UTC
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 22:13:17 UTC
Thanks Chitlesh, for the review and for sponsoring me!

Comment 13 Thomas Moschny 2008-01-22 22:53:37 UTC
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 23:36:48 UTC
cvs done.

Comment 15 Fedora Update System 2008-01-24 21:46:17 UTC
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 21:48:52 UTC
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 14:09:03 UTC
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 14:46:08 UTC
(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 15:11:06 UTC
> 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 11:28:20 UTC
(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 13:41:44 UTC
> Can this easily be reverted when there is a KDE4 port?

yes.

Comment 22 Fedora Update System 2008-02-07 20:58:51 UTC
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 20:59:35 UTC
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.