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

(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

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.

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. ;-/

(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 :-)

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!

spec file updated. Thanks for your input.

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 ?

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.

(In reply to comment #9)
> I am not sponsored by anyone.

Well, please check:
http://fedoraproject.org/wiki/Extras/HowToGetSponsored

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.

By the way is any sponsor watching this review? If not,
I can be a candidate who will sponsor you.

(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

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> 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> 0.31-2
- 0.31

Assiging to me.

(In reply to comment #14)
> This means that you have to write like:
> 
> * Tue Nov 21 2006 Shidai Liu, Leo <sdl.web> 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> 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

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.

(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?

Another issue is, themes have no version number and usually they are updated
when the engine is updated.

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. 

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.

(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

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

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

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.

(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.

ping?

(In reply to comment #27)
> ping?

Got too busy these days. I might have to postpone the work until after
Christmas. Sorry.

(In reply to comment #28)
> Got too busy these days. I might have to postpone the work until after
> Christmas. Sorry.

Okay.

(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

(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...

(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

Well, for me accessing to http://cimi.netsons.org/ gets 404
so currently I cannot check if the source files is valid...

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?

Done.

http://www.srcf.ucam.org/~sl392/fedora/SRPMs/gtk-murrine-engine-0.41-1.leof6.src.rpm

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.

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?

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).

Does not need to block FE-NEEDSPONSOR anymore

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:

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?

Package Change Request
======================
Package Name: gtk-murrine-engine
New Branches: EL-4, EL-5, EL-6
Owners: mso

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

CVS done (by process-cvs-requests.py).

Builds for epel5 and epel6 are now done. Epel4 alas has too old gtk2.