Bug 234711 - Review Request: drapes - A wallpaper manager application for the GNOME desktop
Review Request: drapes - A wallpaper manager application for the GNOME desktop
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jef Spaleta
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-31 13:11 EDT by Damien Durand
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-13 10:16:32 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
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 03:40 EDT, Jef Spaleta
no flags Details

  None (edit)
Description Damien Durand 2007-03-31 13:11:13 EDT
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 03:38:41 EDT
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 03:40:29 EDT
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 06:50:11 EDT
* 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 00:50:03 EDT
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 14:04:44 EDT
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 15:32:36 EDT
CVS done.

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