Bug 215224 - Review Request: gtk-murrine-engine - Murrine GTK2 engine
Review Request: gtk-murrine-engine - Murrine GTK2 engine
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-12 11:23 EST by Leo
Modified: 2010-08-01 10:06 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-15 13:43:31 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)
my review of the package (9.17 KB, text/plain)
2006-11-16 15:10 EST, Thomas M Steenholdt
no flags Details

  None (edit)
Description Leo 2006-11-12 11:23:52 EST
Spec URL: http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine.spec
SRPM URL: http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.31-1.leofc5.src.rpm
Description: Murrine is a cairo-based fast gtk2 theme engine.
Comment 1 Thomas M Steenholdt 2006-11-16 07:17:18 EST
I'm running the gtk-murrine-engine-0.31-1.leofc6 package and it's working great
for me... I know there are certain review procedures, but i'm not familiar with
these - If you guys need me to try to go through it all, could you please
provide a link? Thanks
Comment 2 Paul Howarth 2006-11-16 08:22:21 EST
(In reply to comment #1)
> I'm running the gtk-murrine-engine-0.31-1.leofc6 package and it's working great
> for me... I know there are certain review procedures, but i'm not familiar with
> these - If you guys need me to try to go through it all, could you please
> provide a link? Thanks

http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 3 Thomas M Steenholdt 2006-11-16 12:58:08 EST
Thanks a lot...

But unless I'm missing something, the Reviewer must currently be maintainer of a
package in core or extras (which I'm not). In that case, please accept my input
that "the package seems to works as expected".

Oh, one other thing - It would actually be really nice, if the package included
a logwatch script that could add summary info on postgrey activities.
Comment 4 Thomas M Steenholdt 2006-11-16 13:46:13 EST
argh - please disregard the last two lines of previous post as a bug mixup!
My apologies.

...And just to clarify - the gtk-murrine-engine package is working perfectly for
me too. ;-/
Comment 5 Paul Howarth 2006-11-16 13:55:11 EST
(In reply to comment #3)
> Thanks a lot...
> 
> But unless I'm missing something, the Reviewer must currently be maintainer of a
> package in core or extras (which I'm not).

Well that may prevent you from being the formal reviewer of the package but it
doesn't stop you going through the checklist and pointing out anything that you
notice (if you wish); that would result in any issues getting found and fixed
before someone came along to formally review the package, which in turn would
speed up the formal review process.

It would also get you some credit with potential sponsors should you some day
want to become a Fedora maintainer :-)
Comment 6 Thomas M Steenholdt 2006-11-16 15:10:54 EST
Created attachment 141408 [details]
my review of the package

There are a few places that I did not review (the places are marked), but this
review should still save some time for the primary reviewer and others...

I do apologize if some kind of review-template is already available - hope this
makes sense anyway!
Comment 7 Leo 2006-11-16 15:33:50 EST
spec file updated. Thanks for your input.
Comment 8 Haïkel Guémar 2006-11-16 15:48:03 EST
Even if you're not a sponsor or neither an Extras maintainers shouldn't stop you
from reviewing packages. It might help accelerating the reviewing process. :o)

Leon have you been sponsored ?
By the way, you should use your real name and not an alias in the spec.

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum (1)
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Spec has needed ldconfig in post and postun
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Headers/static libs in -devel subpackage.
OK - .pc files in -devel subpackage.
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed.
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output. (2)

SHOULD Items:
OK - Should include License or ask upstream to include it.
OK - Should build in mock.



1. I haven't found download link for these themes MurrinaBlack and MurrinaTango
on murrine website. Could you provide them in order to check md5sum ?
Is there any reason not to package other themes ?

2. you should think about splitting the package, something like that
- gtk-murrine-engine: the gtk engine (plus maybe the configurator script
available on the website ?)
- gnome-themes-murrine-bigpack: gnome murrine themes pack
It will ease your future work as maintainer.

3. It lacks rpmlint output:
$ rpmlint -i gtk-murrine-engine-0.31-1.leofc5.i386.rpm
W: gtk-murrine-engine summary-ended-with-dot Murrine GTK2 engine.
Summary ends with a dot.

W: gtk-murrine-engine incoherent-version-in-changelog 0.30.2 0.31-1
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

E: gtk-murrine-engine script-without-shebang
/usr/share/themes/Murrina-Black/gtk-2.0/gtkrc
This text file has executable bits set or is located in a path dedicated
for executables, but lacks a shebang and cannot thus be executed.  If the file
is meant to be an executable script, add the shebang, otherwise remove the
executable bits or move the file elsewhere.

These are easily fixable.


Has anyone tested the package on another supported hardware platforms such as
x86_64 ?
Comment 9 Leo 2006-11-16 17:00:30 EST
Updated spec: http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine.spec
Updated SRPM:
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.31-2.leofc5.src.rpm

I am not sponsored by anyone.

1. Themes are downloaded from gnome-look.org. The tree themes included are
chosen for their popularity.
MurrinaTango: http://www.gnome-look.org/content/show.php?content=47709
Murrina-Black: http://www.gnome-look.org/content/show.php?content=46287
MurrinaGilouche: http://www.gnome-look.org/content/show.php?content=44510

2. Splitting seems unnecessary.

3. All fixed.
Comment 10 Mamoru TASAKA 2006-11-21 10:07:50 EST
(In reply to comment #9)
> I am not sponsored by anyone.

Well, please check:
http://fedoraproject.org/wiki/Extras/HowToGetSponsored
Comment 11 Mamoru TASAKA 2006-11-21 10:54:53 EST
Well, just a quick look at this package.

A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* BuildRequires:
  - cairo-devel fontconfig-devel freetype-devel
    All these are uncessary as gtk2-devel requires them.

* rpmlint
  - is not silent.
----------------------------------------------------------------------
W: gtk-murrine-engine incoherent-version-in-changelog 0.31 0.31-2.fc7
----------------------------------------------------------------------
   Please make version-release consistent.
   NOTE: the part of dist tag (i.e. '.fc7') is not needed to be written
   in %changelog.

* BuildRoot
  - %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) is
     recommended. 

* File and Directory Ownership
   - /usr/share/themes
     is not owned by any packages needed by this package.
     * if this package requires gtk2-engines, please add it to Requires.
     * if not, please have this package own the directory.

B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
* Source:
  - Please specify the URL.

C. Other things:

* Themes
  - I cannot understand why you chose the 3 themes. I don't think that
    'their popularity' is a good reason because many people (including
    me) cannot judge how 'popular' they are. 
    My thought is that you have to include _all_ themes available to 
    avoid arbitrariness or choose one (or some) theme(s) with 
    somewhat definitive reason.

    As far as I read http://cimi.netsons.org/pages/murrine/themes.php ,
    http://cimi.netsons.org/media/download_gallery/MurrineThemePack.tar.bz2
    seems the best as it says 'First Theme Pack for the 
    Murrine Gtk2 Cairo Engine'

I only checked for packaging issue.
Comment 12 Mamoru TASAKA 2006-11-21 10:57:23 EST
By the way is any sponsor watching this review? If not,
I can be a candidate who will sponsor you.
Comment 13 Leo 2006-11-21 14:10:22 EST
(In reply to comment #11)
> Well, just a quick look at this package.
> 
> A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> * BuildRequires:
>   - cairo-devel fontconfig-devel freetype-devel
>     All these are uncessary as gtk2-devel requires them.

Corrected with only gtk2-devel.

> 
> * rpmlint
>   - is not silent.
> ----------------------------------------------------------------------
> W: gtk-murrine-engine incoherent-version-in-changelog 0.31 0.31-2.fc7
> ----------------------------------------------------------------------
>    Please make version-release consistent.
>    NOTE: the part of dist tag (i.e. '.fc7') is not needed to be written
>    in %changelog.

Don't know what to do with this. It seems I have no .fc7 in log entries.

> 
> * BuildRoot
>   - %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) is
>      recommended. 

corrected.

> 
> * File and Directory Ownership
>    - /usr/share/themes
>      is not owned by any packages needed by this package.
>      * if this package requires gtk2-engines, please add it to Requires.
>      * if not, please have this package own the directory.
> 
> B. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
> * Source:
>   - Please specify the URL.

Done.

> 
> C. Other things:
> 
> * Themes
>   - I cannot understand why you chose the 3 themes. I don't think that
>     'their popularity' is a good reason because many people (including
>     me) cannot judge how 'popular' they are. 
>     My thought is that you have to include _all_ themes available to 
>     avoid arbitrariness or choose one (or some) theme(s) with 
>     somewhat definitive reason.
> 
>     As far as I read http://cimi.netsons.org/pages/murrine/themes.php ,
>     http://cimi.netsons.org/media/download_gallery/MurrineThemePack.tar.bz2
>     seems the best as it says 'First Theme Pack for the 
>     Murrine Gtk2 Cairo Engine'

themes from replaced with all Murrine themes from the author's website.

> 
> I only checked for packaging issue.


Thank you very much for your input.

Updated files:
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine.spec
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.31-3.leofc5.src.rpm
Comment 14 Mamoru TASAKA 2006-11-21 14:48:28 EST
Before checking 0.31-3:

(In reply to comment #13)
> > * rpmlint
> >   - is not silent.
> > ----------------------------------------------------------------------
> > W: gtk-murrine-engine incoherent-version-in-changelog 0.31 0.31-2.fc7
> > ----------------------------------------------------------------------
> >    Please make version-release consistent.
> >    NOTE: the part of dist tag (i.e. '.fc7') is not needed to be written
> >    in %changelog.
> 
> Don't know what to do with this. It seems I have no .fc7 in log entries.
This means that you have to write like:

* Tue Nov 21 2006 Shidai Liu, Leo <sdl.web@gmail.com> 0.31-3
- remove themes from gnome-look
- remove CREDITS patch
- add all themes from upstream

* Thu Nov 16 2006 Shidai Liu, Leo <sdl.web@gmail.com> 0.31-2
- 0.31
Comment 15 Mamoru TASAKA 2006-11-21 14:50:19 EST
Assiging to me.
Comment 16 Leo 2006-11-21 18:11:13 EST
(In reply to comment #14)
> This means that you have to write like:
> 
> * Tue Nov 21 2006 Shidai Liu, Leo <sdl.web@gmail.com> 0.31-3
> - remove themes from gnome-look
> - remove CREDITS patch
> - add all themes from upstream
> 
> * Thu Nov 16 2006 Shidai Liu, Leo <sdl.web@gmail.com> 0.31-2
> - 0.31
> 

Done!
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine.spec
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.31-3.leofc5.src.rpm
Comment 17 Haïkel Guémar 2006-11-22 02:55:36 EST
Splitting is not unnecessary, you should reconsider this issue.
It will avoid to update the whole package when either one of the theme or the
engine gets updated. Others gtk-engines packaged in Extras do the same.
Comment 18 Leo 2006-11-22 08:17:47 EST
(In reply to comment #17)
> Splitting is not unnecessary, you should reconsider this issue.
> It will avoid to update the whole package when either one of the theme or the
> engine gets updated. Others gtk-engines packaged in Extras do the same.

Agree.

Does this mean I need to package the themes in another SRPM and submit it for
review?
Comment 19 Leo 2006-11-22 08:19:25 EST
Another issue is, themes have no version number and usually they are updated
when the engine is updated.

Comment 20 Mamoru TASAKA 2006-11-22 10:14:35 EST
My thought is:

1. Providing some package without any version usually means
  'this package won't be updated'. 

2. Splitting noarch component is highly recommend when
   - the noarch component is updated frequently
   - or the component is 'rather' large
   I think the themes included in this package doesn't seem
   to fulfill neither condition and I see little benefit for
   splitting themes.

(In reply to comment #19)
> Another issue is, themes have no version number and usually 
> they are updated
> when the engine is updated.
Also in this case, there is little benefit for splitting themes.

However, you can ask upstream if there is any plan that 
the themes you included in this srpm will be updated. 

Comment 21 Mamoru TASAKA 2006-11-22 11:12:19 EST
Well,

* By the way, what is the license of the themes you
  included in srpm? Would you ask upstream about this?

* Also, will you ask upsteam as of the update plan of 
  themes?

* Changelog:
  - version-release (0.31-4) and Changelog (0.31-3) is incoherent.
Comment 22 Leo 2006-11-24 09:47:01 EST
(In reply to comment #21)
> Well,
> 
> * By the way, what is the license of the themes you
>   included in srpm? Would you ask upstream about this?
> 
> * Also, will you ask upsteam as of the update plan of 
>   themes?

A second email to upstream sent.

> 
> * Changelog:
>   - version-release (0.31-4) and Changelog (0.31-3) is incoherent.

Corrected.
http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.31-4.leofc5.src.rpm
Comment 23 Mamoru TASAKA 2006-11-24 12:11:00 EST
Well, I have not yet checked your new srpm, however
I think it is time I should write the following note.

--------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
should sponsor you.

Once you are sponsored, you have the right to formally review other 
submitters' review request and approve the packages. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines".

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" (at the time you are not sponsored, you cannot do
   a formal review) of other person's review request.

When you submitted a new review request or have pre-reviewed other person's
review request, please write the bug number on this bug report so that I
can check your comments or review request.
Fedora Extras package review requests which are waiting for someone to review
can be checked on:
https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-NEW&hide_resolved=1

And Please check the details on
http://fedoraproject.org/wiki/Extras/HowToGetSponsored
Comment 24 Mamoru TASAKA 2006-11-24 12:19:24 EST
Another note:

-------------------------------------------------------------------
Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
Comment 25 Mamoru TASAKA 2006-11-26 04:07:29 EST
Well, this package (gtk-murrine-engine) is okay.
Then the last issue remained is that you have to get sponsored.

Please follow my comment #23 and comment #24 .
When you have pre-reviewed or submitted other package, please let
me know.
Comment 26 Leo 2006-11-26 09:23:32 EST
(In reply to comment #25)
> Well, this package (gtk-murrine-engine) is okay.
> Then the last issue remained is that you have to get sponsored.
> 
> Please follow my comment #23 and comment #24 .
> When you have pre-reviewed or submitted other package, please let
> me know.

I'll submit one on 2 Dec.
Comment 27 Mamoru TASAKA 2006-12-10 12:33:58 EST
ping?
Comment 28 Leo 2006-12-10 12:57:35 EST
(In reply to comment #27)
> ping?

Got too busy these days. I might have to postpone the work until after
Christmas. Sorry.
Comment 29 Mamoru TASAKA 2006-12-10 13:04:53 EST
(In reply to comment #28)
> Got too busy these days. I might have to postpone the work until after
> Christmas. Sorry.

Okay.
Comment 30 Leo 2007-01-10 01:39:51 EST
(In reply to comment #27)
> ping?

I have submitted another package for review.

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

BTW:
gtk-murrine-engine 0.40.1 RPM for fc6
http://www.srcf.ucam.org/~sl392/fedora/zod/gtk-murrine-engine-0.40.1-1.leof6.i386.rpm
Comment 31 Mamoru TASAKA 2007-01-10 12:52:15 EST
(In reply to comment #30)
> gtk-murrine-engine 0.40.1 RPM for fc6
>
http://www.srcf.ucam.org/~sl392/fedora/zod/gtk-murrine-engine-0.40.1-1.leof6.i386.rpm

Please tell me the place for srpm...
Comment 32 Leo 2007-01-10 13:02:02 EST
(In reply to comment #31)
> (In reply to comment #30)
> > gtk-murrine-engine 0.40.1 RPM for fc6
> >
>
http://www.srcf.ucam.org/~sl392/fedora/zod/gtk-murrine-engine-0.40.1-1.leof6.i386.rpm
> 
> Please tell me the place for srpm...

http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.40.1-1.leof6.src.rpm
Comment 33 Mamoru TASAKA 2007-01-11 12:20:53 EST
Well, for me accessing to http://cimi.netsons.org/ gets 404
so currently I cannot check if the source files is valid...
Comment 34 Mamoru TASAKA 2007-01-12 02:48:42 EST
Umm.. still I cannot connect to http://cimi.netsons.org/, however,
when I logged in to other host and tried to connect there from the
host, it suceeded....

Then, 0.41 seems released, so would you update your srpm?
Comment 36 Mamoru TASAKA 2007-01-12 10:56:23 EST
Okay.
= All seven sources coincise with upstream as of md5sum value.
= Packaging are okay.
= Well, your another review request (bug 222087) seems to have several
  issues, however, I believe someone points out them and
  you will fix up soon (I am also reviewing several review
  requests and reviewing your another requests may take long....)

-----------------------------------------------------------------
     This package (gtk-murrine-engine) is APPROVED by me.
------------------------------------------------------------------
Please step forward according to
http://fedoraproject.org/wiki/Extras/Contributors .

When you follow some procedure, I should receive a mail
that you need a sponsor. Then I will sponsor you.
Comment 37 Leo 2007-01-13 03:50:14 EST
Tried to 'make build' gtk-murrine-engine but failed.

----------------------
/usr/bin/plague-client build gtk-murrine-engine gtk-murrine-engine-0_41-1_fc7 devel
Server returned an error: Insufficient privileges.
----------------------

Any ideas?
Comment 38 Mamoru TASAKA 2007-01-15 13:33:59 EST
Please close this bug as CLOSED NEXTRELEASE when
rebuilding is done (I can see this package imported
into all repos so I think you can close this bug).
Comment 39 Till Maas 2007-01-26 14:42:32 EST
Does not need to block FE-NEEDSPONSOR anymore
Comment 40 uokrent 2010-07-16 11:02:53 EDT
Hey, I followed the directions I found here: http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure#other

It says to set the fedora-cvs flag, but it seems I'm not able to do that...

Package Change Request
======================
Package Name: gtk-murrine-engine
New Branches: EL-6
Owners:
Comment 41 Mamoru TASAKA 2010-07-16 11:27:01 EDT
Hello, uokrent:

- First of all, did you contact current gtk-murrine-engine
  maintainer on Fedora (rawhide) and he agreed that you would
  be the maintainer on EPEL6?

- Also, a person who maintain packages on Fedora must be "packager"
  group on Fedora Account System. Are you a member of packager group
  on FAS?
Comment 42 Martin Sourada 2010-07-19 13:06:22 EDT
Package Change Request
======================
Package Name: gtk-murrine-engine
New Branches: EL-4, EL-5, EL-6
Owners: mso
Comment 43 Martin Sourada 2010-07-19 13:10:42 EDT
Ok, I'll maintain this package for EPEL as well.

uokrent: As I said in an earlier e-mail to you, I'll welcome you as comaintainer for any of the EPEL branches. You need to be a member of packager group first, though. See:
http://fedoraproject.org/wiki/PackageMaintainers/Join
Comment 44 Kevin Fenzi 2010-07-19 23:12:04 EDT
CVS done (by process-cvs-requests.py).
Comment 45 Martin Sourada 2010-08-01 10:06:04 EDT
Builds for epel5 and epel6 are now done. Epel4 alas has too old gtk2.

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