Bug 663092 (oxygen-gtk) - Review Request: oxygen-gtk - Oxygen GTK theme
Summary: Review Request: oxygen-gtk - Oxygen GTK theme
Keywords:
Status: CLOSED ERRATA
Alias: oxygen-gtk
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dmitrij S. Kryzhevich
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-12-14 17:03 UTC by Rex Dieter
Modified: 2011-12-08 18:19 UTC (History)
6 users (show)

Fixed In Version: oxygen-gtk-1.0.1-1.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-21 23:02:42 UTC
Type: ---
Embargoed:
kryzhev: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)
fix to combobox (67 bytes, text/plain)
2011-01-05 18:17 UTC, Magnus Tuominen
no flags Details

Description Rex Dieter 2010-12-14 17:03:06 UTC
Spec URL: http://rdieter.fedorapeople.org/rpms/oxygen-gtk/oxygen-gtk.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/oxygen-gtk/oxygen-gtk-1.0.0-1.fc13.src.rpm
Description:
Oxygen-Gtk is a port of the default KDE widget theme (Oxygen), to gtk.

It's primary goal is to ensure visual consistency between gtk-based and
qt-based applications running under KDE. A secondary objective is to also
have a stand-alone nice looking gtk theme that would behave well on other
Desktop Environments.

Unlike other attempts made to port the KDE oxygen theme to gtk, this
attempt does not depend on Qt (via some Qt to Gtk conversion engine),
nor does render the widget appearance via hard-coded pixmaps, which
otherwise breaks every time some setting is changed in KDE.

Comment 1 Jason Tibbitts 2010-12-14 17:19:18 UTC
Is this related to bug 642238?

Comment 2 Rex Dieter 2010-12-14 17:28:13 UTC
No, it is not.

Comment 3 Mohamed El Morabity 2010-12-15 15:40:12 UTC
cairo-devel as BR is useless, since it is already required by gtk2-devel.

Comment 4 Dmitrij S. Kryzhevich 2010-12-22 06:49:36 UTC
And this is a review.

Good.
===========
# rpmlint oxygen-gtk-1.0.0-1.fc14.src.rpm oxygen-gtk-1.0.0-1.fc14.x86_64.rpm 
oxygen-gtk.src: W: spelling-error %description -l en_US pixmaps -> pix maps, pix-maps, bitmaps
oxygen-gtk.x86_64: W: spelling-error %description -l en_US pixmaps -> pix maps, pix-maps, bitmaps
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

rpmlint output could be ignored.
The package is named according to the Package Naming Guidelines.
The spec file name must match the base package %{name}. 

-------
Spec name correct, but. Gtk engines are named as gtk-NAME-engine. Could we rename package into gtk-oxygen-engine? There are no guides for it, just common usage.
-------

* The package meets the Packaging Guidelines.
* The package is licensed with a Fedora approved license and meet the Licensing Guidelines.
* The License field in the package spec file matchs the actual license.
* File, containing the text of the license for the package, included in %doc.
* The spec file is written in American English.
* The spec file for the package is legible.
* The sources used to build the package matchs the upstream source with md5sum 441398b4569ce0282c39e5c21cb16dfc.
* The package built on F14 x86_64.
* There are no locales.
* There are no need in ldconfig.
* There are no bundle copies of system libraries.
* A package owns all directories that it creates.
* All files are not listed more than once in the spec file's %files listings.
* Permissions on files are set properly.
* Package consistently use macros.
* The package contains code.
* There are no large documentation
* Everything included as %doc does not affect the runtime of the application.
* No need in -devel.
* Package does not own files or directories already owned by other packages.
* All filenames in rpm packages are valid UTF-8.

SHOULD:
* The package built in mock.
* The package works as described.

Not so good
=======
1) All required build dependencies are listed in BuildRequires, but cairo-devel is dependence for gtk2-devel and not needed to be call explicitly.
2) Runtime dependence. We can't be sure there are all needed icons installed with gtk2. Main icon theme is hicolor-icons-theme, if we have gtk2, we have it. But this theme containe not all icons. oxygen-icon-theme, ie, containe them. {gnome,nuvolla,...}-icon-theme - the same, but not hicolor-i-t. Somehow we need anything to make sure - we have sufficient icon set.
Second is not a blocker.

Comment 5 Rex Dieter 2011-01-03 15:57:35 UTC
re: naming
I'd rather not rename, to avoid confusion with another similar project, see bug #642238

otherwise, you consider an extraneous BR a review blocker?  OK, I'll remove cairo-devel

Comment 6 Rex Dieter 2011-01-03 16:03:56 UTC
The icon thing *should* be ok as-is, my understanding is that oxygen-gtk simply checks for the existence of any icon-theme preference in kde's kdeglobals, and uses that, if present.

Comment 7 Rex Dieter 2011-01-03 16:10:09 UTC
Spec URL: http://rdieter.fedorapeople.org/rpms/oxygen-gtk/oxygen-gtk.spec
SRPM URL:
http://rdieter.fedorapeople.org/rpms/oxygen-gtk/oxygen-gtk-1.0.0-2.fc14.src.rpm

%changelog
* Mon Jan 03 2011 Rex Dieter <rdieter> - 1.0.0-2
- drop extraneous BR: cairo-devel

Comment 8 Jaroslav Reznik 2011-01-04 09:12:44 UTC
(In reply to comment #5)
> re: naming
> I'd rather not rename, to avoid confusion with another similar project, see bug
> #642238
> 
> otherwise, you consider an extraneous BR a review blocker?  OK, I'll remove
> cairo-devel

Wouldn't be better to have only ONE oxygen gtk engine? And I think native one should be preferred solution for us instead of hacks painting widgets by other toolkit.

Comment 9 Magnus Tuominen 2011-01-05 18:17:53 UTC
Created attachment 471919 [details]
fix to combobox

Uploading patch to fix KDE bug #261971

Comment 10 Dmitrij S. Kryzhevich 2011-01-10 06:01:22 UTC
Sorry for delay.

Rename: you said "no", OK.
BR: OK.

=============
APPROVED.
=============

Comment 11 Rex Dieter 2011-01-11 16:35:06 UTC
Thanks!

New Package SCM Request
=======================
Package Name: oxygen-gtk
Short Description: Oxygen GTK theme
Owners: rdieter
Branches: f13 f14 el6
InitialCC:

Comment 12 Jason Tibbitts 2011-01-11 18:19:46 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2011-01-11 19:33:34 UTC
oxygen-gtk-1.0.0-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/oxygen-gtk-1.0.0-2.fc14

Comment 14 Fedora Update System 2011-01-11 19:34:08 UTC
oxygen-gtk-1.0.0-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/oxygen-gtk-1.0.0-2.fc13

Comment 15 Fedora Update System 2011-01-13 23:34:40 UTC
oxygen-gtk-1.0.1-1.fc13 has been pushed to the Fedora 13 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 oxygen-gtk'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/oxygen-gtk-1.0.1-1.fc13

Comment 16 Fedora Update System 2011-01-21 23:02:36 UTC
oxygen-gtk-1.0.1-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2011-01-21 23:08:31 UTC
oxygen-gtk-1.0.1-1.fc14 has been pushed to the Fedora 14 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.