Bug 234711 - Review Request: drapes - A wallpaper manager application for the GNOME desktop
Summary: Review Request: drapes - A wallpaper manager application for the GNOME desktop
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jef Spaleta
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-31 17:11 UTC by Damien Durand
Modified: 2007-11-30 22:12 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-13 14:16:32 UTC
Type: ---
jspaleta: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)
Updated drapes specfile which includes all suggested fixes for review (2.99 KB, text/plain)
2007-04-13 07:40 UTC, Jef Spaleta
no flags Details

Description Damien Durand 2007-03-31 17:11:13 UTC
Spec URL: http://glive.tuxfamily.org/fedora/desktop-drapes/desktop-drapes.spec
SRPM URL: http://glive.tuxfamily.org/fedora/desktop-drapes/desktop-drapes-0.5.1-1.src.rpm
Description: Desktop drapes is a wallpaper manager application for the gnome desktop. 
It can randomly changing your wallpaper every once in a while, or 
whenever you fell like it. It can also automaticaly pickup any 
wallpapers you added to a directory with the ninja magic of inotify

Comment 1 Jef Spaleta 2007-04-13 07:38:41 UTC
Here are the list of blocker review issues as I see it. I have provided an
updated specfile as an attachment to this report which I believe address each of
the issues below. If you have an issue with any of the suggested changes, please
let me know.


Review items that need to be addressed:
- Change the name to drapes to match upstream tarball and packaging
... Fixed in updated spec
- Need BuildRequires: perl-XML-Parser to build under mock against development tree
... Fixed in updated spec
- Directory ownership issues:
  Package should require hicolor-icon-theme for /usr/share/icons/hicolor/*/apps/
ownership
  Package should require yelp for /usr/share/gnome/help/ ownership
  Package should require libbonobo for /usr/lib/bonobo/servers/ ownership
  Package should own /usr/share/omf/drapes/ and /usr/share/gnome/help/drapes/
and /usr/lib/drapes/
... Fixed in updated spec
- Minor Scriptlet changes to bring the package inline with 
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
... Fixed in updated spec

Details concerning scriptlets:
+ GConf scriptlets and requires look good according to ScriptletSnippets
- scrollkeeper scriptlet updated for to best practises from ScriptletSnippets
... updated spec file contains suggested changes.
- update-desktop-database scriptlets technically not needed, since desktop file
does not reference a MimeType.
... removed in the updates specfile.


Non-blocker item:
- Close button on the About dialog doesn't work.
... This is not a blocker, but should be filed with upstream developer.


Items which pass review:
+ rpmlint not clean, but messages appear to be bogus
pmlint messages from binary with comments inline:
E: desktop-drapes no-binary
  I'm not sure what this means. This looks bogus to me. The /usr/bin/drapes file
is a bash script which shells out to mono. I don't see a problem here.
E: desktop-drapes only-non-binary-in-usr-lib
  I think this is also bogus, the file location /usr/lib/drapes/drapes.exe
appears to be consistent with other mono application packaging such as tomboy.
W: desktop-drapes non-conffile-in-etc /etc/gconf/schemas/drapes.schemas
  clearly a bogus warning

+ Specfile name matches name field. Note that both should probably change to
just 'drapes'
+ URL and SOURCE0 tags look good
+ md5sum of included source matches with upstream source
3ae3b1489f33a3e3b6ccaa3dd8782622  drapes-0.5.1.tar.gz
+ GPL License tag matches actual license as listed in source COPYING file.
COPYING file is included in docs section appropriately
+ specfile is in legible english-ese
+ no need for ldconfig no shared libraries
+ locales seem to be handled correctly
+ files section looks good ( after correction for directory ownership )
+ permissions look good
+ clean section looks good
+ install section looks good
+ no need for doc subpackage
+ no need for devel subpackage
+ docs section does not include runtime necessary files
+ desktop file is included and appears to be installed correctly
  (Suggestion, add fedora as vendor-id.. this is included in updated spec)
+ build process appears to be using the rpm build environment compiler options
correctly.




Comment 2 Jef Spaleta 2007-04-13 07:40:29 UTC
Created attachment 152519 [details]
Updated drapes specfile which includes all suggested fixes for review

This specfile includes all suggested review blocker fixes.
Most importantly this includes a name change from desktop-drapes to drapes
to bring it inline with upstream packaging.

Comment 3 Damien Durand 2007-05-29 10:50:11 UTC
* Thu May 29 2007 Damien Durand <splinux@fedoraproject.org> - 0.5.1-3
- Fix setup part, rename to drapes

New spec: http://glive.tuxfamily.org/fedora/drapes/drapes.spec
New srpms: http://glive.tuxfamily.org/fedora/drapes/drapes-0.5.1-3.fc7.src.rpm

Is it ok?

Comment 4 Jef Spaleta 2007-05-30 04:50:03 UTC
Here's the good news, everything I found the first time looks good.

Here's the bad news, this is a mono application... you need to add an additional
Requires: gnome-sharp

gnome-sharp will pull in mono-core and also gtk-sharp2.

I'm pretty sure from testing on my system that this application doesn't need
mono-web or another mono component beyond mono-core,

I'm going to approve this. Please make sure you add the requires for gnome-sharp
to the specfile before you build this in the koji buildsystem.

-jef


Comment 6 Damien Durand 2007-05-31 18:04:44 UTC
New Package CVS Request
=======================
Package Name: drapes
Short Description: A wallpaper manager application for the GNOME desktop
Owners: splinux@fedoraproject.org
Branches: FC-6 F-7
InitialCC: splinux25@gmail.com

Comment 7 Jason Tibbitts 2007-06-01 19:32:36 UTC
CVS done.


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