Bug 462250 - Review Request: python-pmw - python megawidgets
Summary: Review Request: python-pmw - python megawidgets
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: PyMOL
TreeView+ depends on / blocked
 
Reported: 2008-09-14 21:00 UTC by Tim Fenn
Modified: 2014-11-06 12:48 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-16 17:18:19 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Tim Fenn 2008-09-14 21:00:15 UTC
spec url: http://www.stanford.edu/~fenn/packs/pmw.spec
srpm url: http://www.stanford.edu/~fenn/packs/pmw-1.3.2-1.f8.src.rpm

Pmw is a toolkit for building high-level compound widgets in Python using the Tkinter module.

It consists of a set of base classes and a library of flexible and extensible megawidgets built on this foundation. These megawidgets include notebooks, comboboxes, selection widgets, paned widgets, scrolled widgets and dialog windows. 

Also see:
http://pmw.sourceforge.net/

Comment 1 Mamoru TASAKA 2008-09-26 07:01:01 UTC
Some notes.

* Build failure
  - Your srpm does not build:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=843287
    * At least python is needed for BuildRequires
    * Also even after python is added to BuildRequires the build fails (on F-9+) like:
----------------------------------------------------------------
error: Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/Pmw-1.3.2-py2.5.egg-info


RPM build errors:
    user fenn does not exist - using root
    Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/Pmw-1.3.2-py2.5.egg-info
-----------------------------------------------------------------

* Redundant definition macro
  - %python_sitearch is used nowhere
  - Also it is not needed to define %version, %release macro explicitly.

* Source URL
  - %SOURCE must be written with full URL:
    https://fedoraproject.org/wiki/Packaging/SourceURL

* The line
-----------------------------------------------------------------
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
-----------------------------------------------------------------
  - is not needed. You define $RPM_BUILD_ROOT.

Comment 2 Mamoru TASAKA 2008-09-26 07:25:30 UTC
* Also I think this srpm should be named as "python-pmw":
  https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28python_modules.29

Comment 3 Tim Fenn 2008-10-01 23:32:53 UTC
Sorry for the late reply, I'm away from home and with a rather spotty connection.

(In reply to comment #1)
> Some notes.
> 
> * Build failure
>   - Your srpm does not build:
>     http://koji.fedoraproject.org/koji/taskinfo?taskID=843287
>     * At least python is needed for BuildRequires
>     * Also even after python is added to BuildRequires the build fails (on
> F-9+) like:
> ----------------------------------------------------------------
> error: Installed (but unpackaged) file(s) found:
>    /usr/lib/python2.5/site-packages/Pmw-1.3.2-py2.5.egg-info
> 

Fixed.

> 
> * Redundant definition macro
>   - %python_sitearch is used nowhere
>   - Also it is not needed to define %version, %release macro explicitly.
> 
> * Source URL
>   - %SOURCE must be written with full URL:
>     https://fedoraproject.org/wiki/Packaging/SourceURL
> 

Fixed.

> * The line
> -----------------------------------------------------------------
> [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
> -----------------------------------------------------------------
>   - is not needed. You define $RPM_BUILD_ROOT.

Fixed.

> * Also I think this srpm should be named as "python-pmw":

Done.

spec url: http://www.stanford.edu/~fenn/packs/python-pmw.spec
srpm url: http://www.stanford.edu/~fenn/packs/python-pmw-1.3.2-2.f8.src.rpm

Comment 4 Mamoru TASAKA 2008-10-02 16:16:33 UTC
For 1.3.2-2:

* URL
  - I think http://pmw.sourceforge.net/ is better for URL

* SourceURL
  - For tarball on sourceforge.net please refer to
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* %prep
---------------------------------------------------
%setup -q -n Pmw.1.3.2/src
---------------------------------------------------
  - After rebuilding the directory Pmw.1.3.2/src under %_builddir
    But it leaved Pmw.1.3.2 directory undeleted, which is not
    right.
    %setup directory (relative to %_builddir) must not contain
    any slash. 

%build

%install
rm -rf 

* License
  --- Some files under src/Pmw/Pmw_1_3/contrib/ are under GPLv2+
  --- Others are under MIT (from src/Pmw/Pmw_1_3/doc/copyright.html)
  - So the license tag should be "MIT and GPLv2+".

* %clean, %install
  - %clean section is empty
     See https://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
     "MUST: Each package must have a %clean section, ...."
  - Also %install does not contain any "cleaning" line.
    https://fedoraproject.org/wiki/Packaging/Guidelines#PreppingBuildRootForInstall

* Directory ownership issue
  - The directory %{python_sitelib}/ itself is owned by python and
    should not be owned by this rpm.

* Documents
  - As license information is in src/Pmw/Pmw_1_3/doc/copyright.html
    at least this file should be included in %doc.
    I guess
-------------------------------------------------
%doc src/Pmw/Pmw_1_3/doc/
-------------------------------------------------
    is better.

Comment 5 Tim Fenn 2008-10-04 03:23:31 UTC
(In reply to comment #4)
> For 1.3.2-2:
> 
> * URL
>   - I think http://pmw.sourceforge.net/ is better for URL
> 
> * SourceURL
>   - For tarball on sourceforge.net please refer to
>     https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
> 

done.

> * %prep
> ---------------------------------------------------
> %setup -q -n Pmw.1.3.2/src
> ---------------------------------------------------
>   - After rebuilding the directory Pmw.1.3.2/src under %_builddir
>     But it leaved Pmw.1.3.2 directory undeleted, which is not
>     right.
>     %setup directory (relative to %_builddir) must not contain
>     any slash. 
> 

fixed.

> 
> * License
>   --- Some files under src/Pmw/Pmw_1_3/contrib/ are under GPLv2+
>   --- Others are under MIT (from src/Pmw/Pmw_1_3/doc/copyright.html)
>   - So the license tag should be "MIT and GPLv2+".
> 

fixed.

> * %clean, %install
>   - %clean section is empty
>      See https://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
>      "MUST: Each package must have a %clean section, ...."
>   - Also %install does not contain any "cleaning" line.
>

fixed.
    
> 
> * Directory ownership issue
>   - The directory %{python_sitelib}/ itself is owned by python and
>     should not be owned by this rpm.
> 

fixed.

> * Documents
>   - As license information is in src/Pmw/Pmw_1_3/doc/copyright.html
>     at least this file should be included in %doc.
>     I guess
> -------------------------------------------------
> %doc src/Pmw/Pmw_1_3/doc/
> -------------------------------------------------
>     is better.

done.

I also fixed the egg-info packaging according to https://fedoraproject.org/wiki/Packaging/Python/Eggs.

spec url: http://www.stanford.edu/~fenn/packs/python-pmw.spec
srpm url: http://www.stanford.edu/~fenn/packs/python-pmw-1.3.2-3.f8.src.rpm

Comment 6 Mamoru TASAKA 2008-10-04 18:12:45 UTC
For -3:

* %version macro
  - Please consider to use %{version} macro (also see:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* CFLAGS in noarch rpm
  - The part CFLAGS="$RPM_OPT_FLAGS" is not needed because
    this is noarch.

* Macros in %changelog or comments
  - In %changelog or comments, use %% instead of % (e.g.
    "add doc to %%files, add ..") so that macros won't be
    expanded.

Now I am trying to see pymol, however it may take a bit long..

Comment 7 Tim Fenn 2008-10-06 18:53:41 UTC
(In reply to comment #6)
> For -3:
> 
> * %version macro
>   - Please consider to use %{version} macro (also see:
>     https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D
> 

I wasn't sure how best to apply this here, can you provide some more detail as to what you mean?

> * CFLAGS in noarch rpm
>   - The part CFLAGS="$RPM_OPT_FLAGS" is not needed because
>     this is noarch.
> 

Oops - fixed.

> * Macros in %changelog or comments
>   - In %changelog or comments, use %% instead of % (e.g.
>     "add doc to %%files, add ..") so that macros won't be
>     expanded.
> 

Done.

spec url: http://www.stanford.edu/~fenn/packs/python-pmw.spec
srpm url: http://www.stanford.edu/~fenn/packs/python-pmw-1.3.2-4.f8.src.rpm

Comment 8 Mamoru TASAKA 2008-10-08 15:28:44 UTC
Okay.

One minor issue
- tkinter has "Requires: python" so "Requires: python" can be removed from this package
  (Requires: tkinter is still needed)

Now:
* This package itself is good
* You have another review request (bug 462251), which I think is almost good
------------------------------------------------------------
    This package (python-pmw) is APPROVED by mtasaka
------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji) ".
I am already sponsoring you.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.

Comment 9 Tim Fenn 2008-10-08 22:58:47 UTC
(In reply to comment #8)
> Okay.
> 
> One minor issue
> - tkinter has "Requires: python" so "Requires: python" can be removed from this
> package
>   (Requires: tkinter is still needed)
> 

Fixed.

spec url: http://www.stanford.edu/~fenn/packs/python-pmw.spec
srpm url: http://www.stanford.edu/~fenn/packs/python-pmw-1.3.2-5.f8.src.rpm

Comment 10 Tim Fenn 2008-10-08 23:01:53 UTC
New Package CVS Request
=======================
Package Name: python-pmw
Short Description: python megawidgets
Owners: timfenn
Branches: F-10 EL-5
InitialCC: timfenn

Comment 11 Huzaifa S. Sidhpurwala 2008-10-09 02:37:29 UTC
cvs done

Comment 12 Mamoru TASAKA 2008-10-16 15:16:28 UTC
ping?

Comment 13 Tim Fenn 2008-10-16 17:10:57 UTC
package has been added to CVS (devel and F-10) according to http://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner , imported and builds requested, all OK.  What else needs to be done?

Comment 14 Mamoru TASAKA 2008-10-16 17:18:19 UTC
Then you can close this bug :)

Comment 15 Tim Fenn 2008-10-20 20:17:48 UTC
please also add F-9 branch (as a requirement to https://bugzilla.redhat.com/show_bug.cgi?id=462251)

Comment 16 Tim Fenn 2008-10-21 18:00:24 UTC
Package Change Request
======================
Package Name: python-pmw
New Branches: F-9
Owners: timfenn

adding python-pmw to allow for F-9 builds of pymol

Comment 17 Kevin Fenzi 2008-10-23 20:36:39 UTC
cvs done.

Comment 18 Tim Fenn 2014-11-06 03:29:05 UTC
Package Change Request
======================
Package Name: python-pmw
New Branches: epel7
Owners: timfenn
InitialCC: timfenn

Comment 19 Gwyn Ciesla 2014-11-06 12:48:19 UTC
Git done (by process-git-requests).


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