Bug 195871 - (obmenu) Review Request: obmenu
Review Request: obmenu
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-06-18 23:13 EDT by Peter Gordon
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-02 14:01:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Gordon 2006-06-18 23:13:25 EDT
Spec URL: http://thecodergeek.com/downloads/fedora/obmenu.spec
Spec URL: http://thecodergeek.com/downloads/fedora/obmenu-1.0-1.src.rpm

Description: obmenu is a graphical Openbox menu editor written in Python,
and also includes an obxml module to use in one's own scripts.
Comment 1 Parag AN(पराग) 2006-06-19 01:21:21 EDT
Review for this package:-
      Mock Build Results for i386 
      - Successfully built for i386  
MUST Items:
     - MUST: rpmlint shows no error 
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package obmenu, in the format
obmenu.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license GPL.
      - MUST: The License field in the package obmenu.spec file matches the
actual license file COPYING in tarball.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains %{__rm} -rf
%{buildroot}.
      - MUST: This package used macros.
      - MUST: Document files are included like README.
    
        This package also followed optimized .pyo files installation successfully.
        You should follow Python Packaging Guidelines for installing module in
pythin_sitelib as
%files
%defattr(-,root,root,-)
%dir %{python_sitelib}/modulename
%{python_sitelib}/modulename/*.py
%{python_sitelib}/modulename/*.pyc
          You have not included/created separate directory for your module.
Comment 2 Parag AN(पराग) 2006-06-19 07:00:49 EDT
Above is Not an official review as I'm not yet sponsored
Comment 3 Kevin Fenzi 2006-09-01 23:15:26 EDT
Thanks for the prelim comments Parag.
Here's a formal review:
OK - Package name
OK - Spec file matches base package name.
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:
710036a5edc9886d6d563ce46c747432  obmenu-1.0.tar.gz
710036a5edc9886d6d563ce46c747432  obmenu-1.0.tar.gz.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
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 doesn't own any directories other packages own.
OK - No rpmlint output.
SHOULD Items:
OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues:

1. The new improved python guidelines require not ghosting, but including
the .pyo files. Can you make that change?

2. You don't use python_sitearch, so might skip defining it at the top.

3. Should this package have a desktop file?
See:
http://www.fedoraproject.org/wiki/Packaging/Guidelines#desktop

4. If I install this package and try and run it, I get:
Error: "/home/kevin/.config/openbox/menu.xml" not found
Should this package then 'Require: openbox' ? Or otherwise
require a menu.xml file?
Comment 4 Peter Gordon 2006-09-02 01:23:22 EDT
Thanks for the preliminary review, Kevin.

(In reply to comment #3)
> Issues:
> 1. The new improved python guidelines require not ghosting, but including
> the .pyo files. Can you make that change?
Fixed in 1.0-2.

> 2. You don't use python_sitearch, so might skip defining it at the top.
I don't think this is really much of a problem per se, but I have removed it in
1.0-2 as suggested.

> 3. Should this package have a desktop file?
> See:
> http://www.fedoraproject.org/wiki/Packaging/Guidelines#desktop
Added in 1.0-2.

> 4. If I install this package and try and run it, I get:
> Error: "/home/kevin/.config/openbox/menu.xml" not found
> Should this package then 'Require: openbox' ? Or otherwise
> require a menu.xml file?
Well, the openbox package does not create a menu.xml file of any sorts in the
user's home directory. However, I'd very much prefer *not* to dink around with
stuff inside of /home as part of a package. For the time being, I've packaged a
 README.Fedora file (as %doc) that contains instructions on copying the default
menu to your home directory. I've also sent an email upstream about this (and
included the text of that in the README.Fedora file). Does this suffice? :)

URLs for 1.0-2 are as follows:
Spec:  http://thecodergeek.com/downloads/fedora/obmenu.spec
SRPM:  http://thecodergeek.com/downloads/fedora/obmenu-1.0-2.src.rpm

Thanks for your time and review!
Comment 5 Kevin Fenzi 2006-09-02 12:47:49 EDT
That all sounds good. All the blockers I was seeing appear to be fixed, 
so this package is APPROVED. 

Don't forget to close this package with NEXTRELEASE when it's been imported and 
built. 
Comment 6 Peter Gordon 2006-09-02 14:01:43 EDT
Built for devel; branch requested for FC-5. Thanks for the review!

(As an aside, I've received a reply from the upstream author that the next
release will feature code attempting to automagically create the user's
configuration directory and copy the default menu.xml to it if it does not yet
exist. Yay!)
Comment 7 Peter Gordon 2007-06-02 17:21:11 EDT
Package Change Request
======================
Package Name: obmenu
Updated Fedora Owners: extras-orphan@fedoraproject.org

I'm orphaning openbox, obconf, and obmenu as I no longer use them and feel that
my time is better spent dedicated to my other packages. Thanks.
Comment 8 Peter Gordon 2007-06-02 17:22:24 EDT
[ Forgot to set fedora-cvs flag. Please see previous comment. ]
Comment 9 Tom "spot" Callaway 2007-06-04 17:54:00 EDT
Orphaned.
Comment 10 Miroslav Lichvar 2007-06-13 04:09:38 EDT
Package Change Request
======================
Package Name: obmenu
Updated Fedora Owners: mlichvar@redhat.com

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