Bug 215169 - Review Request: xfce4-dict-plugin - A XFCE panel plugin to query a Dict server
Review Request: xfce4-dict-plugin - A XFCE panel plugin to query a Dict server
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-11 15:16 EST by Christoph Wickert
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-12 21:19:49 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Christoph Wickert 2006-11-11 15:16:47 EST
Spec URL: http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-dict-plugin.spec
SRPM URL: http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-dict-plugin-0.2.0-1.fc7.src.rpm
Description: With this plugin you can query a dictionary server (see RFC 2229) to search for the translation or explanation of a word. You can also choose a dictionary offered by the server to improve your search results.
Comment 1 Kevin Fenzi 2006-11-11 22:26:18 EST
Here's a review: 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
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:
ac2f3626858b7b6800ac43cc9de6e0bc  xfce4-dict-plugin-0.2.0.tar.bz2
ac2f3626858b7b6800ac43cc9de6e0bc  xfce4-dict-plugin-0.2.0.tar.bz2.1
OK - BuildRequires correct
OK - Spec handles locales/find_lang
See below - 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.

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.
x86_64/i386 - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version

Issues:

1. Should add a:
%defattr(-,root,root,-)
to files?

2. Should the BuildRequire for xfce4-panel-devel 4.3.99.1 be 4.3.99.2,
since everything else is requiring that version?
Comment 2 Christoph Wickert 2006-11-12 08:52:55 EST
Thanks for reviewing this so quickly. I was to tired yesterday to reply.

(In reply to comment #1)

> 1. Should add a:
> %defattr(-,root,root,-)
> to files?

Of course. I have no idea how I managed to delete this line...

> 2. Should the BuildRequire for xfce4-panel-devel 4.3.99.1 be 4.3.99.2,
> since everything else is requiring that version?

No, that's intentional, cause this is what ./configure checks for.

> checking for libxfcegui4-1.0 >= 4.3.90.2... 4.3.99.1
> ...
> checking for libxfce4util-1.0 >= 4.3.90.2... 4.3.99.1
> ...
> checking for libxfce4panel-1.0 >= 4.3.99.1... 4.3.99.1

I usually make the BuildRequires: as low as possible (to make the packages
easier to rebuild) but the Requires: to the version the plugin was compiled for.

Updated Package:
* Sun Nov 12 2006 Christoph Wickert <fedora christoph-wickert de> - 0.2.0-2
- Add %%defattr (#215169).

SPREC:
http://home.arcor.de/christoph.wickert/fedora/extras/review/SPECS/xfce4-dict-plugin.spec
SRPM: 
http://home.arcor.de/christoph.wickert/fedora/extras/review/SRPMS/xfce4-dict-plugin-0.2.0-2.fc7.src.rpm
Comment 3 Kevin Fenzi 2006-11-12 20:48:50 EST
ok. Looks good to me.The blockers look all solved... this package is APPROVED. 

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

Also, consider doing a review of a waiting package to help spread out the
reviewing load.
Comment 4 Christoph Wickert 2006-11-12 21:19:49 EST
(In reply to comment #3)
> Also, consider doing a review of a waiting package to help spread out the
> reviewing load.
I have allready assigned bug #190213 and bug #188542 to me. Will see what I can
do after that.

Imported and built for devel, cvs-sync for FE6 is requested. Closing

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