Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
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
jspaleta: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)
Updated drapes specfile which includes all suggested fixes for review (deleted)
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.