Bug 219981

Summary:	Review Request: xfce4-xfapplet-plugin - A plugin to use gnome-panel based applets inside the Xfce4 one
Product:	[Fedora] Fedora	Reporter:	Christoph Wickert <christoph.wickert>
Component:	Package Review	Assignee:	Kevin Fenzi <kevin>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Package Reviews List <fedora-package-review>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2006-12-22 23:09:14 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	163779

Description Christoph Wickert 2006-12-18 02:27:21 UTC

Spec URL: http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-xfapplet-plugin.spec
SRPM URL: http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-xfapplet-plugin-0.1.0-1.fc7.src.rpm
Description: The XfApplet Plugin is a plugin for the Xfce 4 Panel which allows one to use applets designed for the Gnome Panel inside the Xfce Panel. You can think of XfApplet as a tiny Gnome Panel that lives inside the Xfce Panel and allows you to show the same applets that the Gnome Panel is capable of showing.

Comment 1 Kevin Fenzi 2006-12-22 04:29:40 UTC

I'd be happy to review this package. 

Expect a full review in a bit.

Comment 2 Kevin Fenzi 2006-12-22 04:57:34 UTC

OK - Spec has consistant macro usage.
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.
OK - Sources match upstream md5sum:
6a06c44b18a97626f44a240ad3bc3244  xfce4-xfapplet-plugin-0.1.0.tar.bz2
6a06c44b18a97626f44a240ad3bc3244  xfce4-xfapplet-plugin-0.1.0.tar.bz2.1
OK - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.

See below - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
i386/x86_64 - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version

Issues:

1. The summary seems a bit odd to me:
"A plugin to use gnome-panel based applets inside the Xfce4 one"
How about:
"A plugin to use gnome-panel based applets inside the Xfce4 panel"

2. Your desktop file needs desktop-file-install:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755

Comment 3 Christoph Wickert 2006-12-22 13:24:00 UTC

(In reply to comment #2)
> Issues:
> 
> 1. The summary seems a bit odd to me:
> "A plugin to use gnome-panel based applets inside the Xfce4 one"
> How about:
> "A plugin to use gnome-panel based applets inside the Xfce4 panel"

Thanks, will change that. As I'm not a native english speaker, I wouldn't have
recognized this.

> 2. Your desktop file needs desktop-file-install:

As I said in Bug #219986 Comment #3 I don't think so. Quoting from the guidelines:
"This is mostly for the sake of menu-editing (which bases off of .desktop
file/path names)."
... but nothing in the "Add Items to the Panel" dialog can be edited.

BTW: Is the above sentence from the guidelines correct? "bases OFF OF .desktop
file" sounds strange to me.

Comment 4 Kevin Fenzi 2006-12-22 16:54:37 UTC

Yeah, desktop-file-install isn't needed here. Sorry about that. 

Oddly, that sentence is ok. English is a weird language. ;) 

I don't see any further blockers here, so this package is APPPROVED. 
You can fix the summary on import. 

Don't forget to close this NEXTRELEASE once it's been imported and built.

Comment 5 Christoph Wickert 2006-12-22 23:09:14 UTC

(In reply to comment #4)
> Oddly, that sentence is ok. English is a weird language. ;) 

No, it's not _that_ weird, German is even worse for non native speakers I think. :)

Glad to have English spell-checking in Firefox 2.0 now.

> You can fix the summary on import. 

Done.

> Don't forget to close this NEXTRELEASE once it's been imported and built. 

Done. Closing.