Bug 427275 - Review Request: python-tgexpandingformwidget - A repeating form widget for TurboGears
Summary: Review Request: python-tgexpandingformwidget - A repeating form widget for T...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-02 18:38 UTC by Rob Crittenden
Modified: 2008-01-18 15:39 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-01-18 15:39:26 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Rob Crittenden 2008-01-02 18:38:06 UTC
Spec URL: http://rcritten.fedorapeople.org/python-tgexpandingformwidget.spec
SRPM URL: http://rcritten.fedorapeople.org/python-tgexpandingformwidget-0.1.3-2.fc7.src.rpm
Description: TGExpandingFormWidget is a type of repeating form widget for TurboGears.
It contains groups of widgets which are repeated on the page. The user is
able to dynamically add or remove widget groups as required before submitting
the form.

Comment 1 Jason Tibbitts 2008-01-16 17:20:27 UTC
A couple of notes:

The URL in the URL: tag doesn't exist;  I'd suggest
http://www.psychofx.com/TGExpandingFormWidget/

I'm having trouble understanding why you'd use the egg instead of the tarball
which upstream provides.  We generally try to stay away from eggs if possible,
since you must take extra care to pick out just the source.

Comment 2 Rob Crittenden 2008-01-16 18:20:58 UTC
I used the python-tgfastdata package, another TG extension, as a template for
creating this and that uses the egg format.

I can fix up the URL, can't believe I missed that. Should I convert this to use
the tar.gz source instead of the egg as well? I have no personal preference.

Comment 3 Jason Tibbitts 2008-01-16 23:19:16 UTC
I'm not a python expert but I asked on IRC earlier today:

[11:16] <tibbs|h> abadger1999: What's your opinion on pulling apart an egg
versus just using the upstream tarball when upstream provides one?

[11:17] <f13> if the .egg has all source code, and no compiled python modules,
it's technically OK.

[11:17] <abadger1999> tibbs|h: I'd much rather use a tarball when available.

[11:17] <f13> however I think we'd prefer to use the tarball, unless that's not
the upstream's preferred method of source distribution.

[11:17] <abadger1999> I don't know if it's a blocker but I can't think of any
reason to prefer the egg.

FYI, agbadger1999 is Toshio, who wrote the Python Eggs guidelines, and f13 is
Jesse Keating.

Frankly I don't know enough to have a personal opinion, but if it's no trouble
it does seem that using the tarball is preferred.

Comment 4 Rob Crittenden 2008-01-17 03:11:06 UTC
Ok, now I remember why I use the egg: it doesn't include a setup.py.

I'll see what it takes to write one.

Comment 5 Rob Crittenden 2008-01-17 04:40:57 UTC
Wrote a setup.py that builds the package from the tar.gz.

Spec URL: http://rcritten.fedorapeople.org/python-tgexpandingformwidget.spec
SRPM URL:
http://rcritten.fedorapeople.org/python-tgexpandingformwidget-0.1.3-3.fc7.src.rpm

Comment 6 Jason Tibbitts 2008-01-17 07:32:58 UTC
You know, now that I look at it, it seems that the tarball just has the same
contents as the egg file, as if they unzipped it and then tarred it up. 
Someone's unclear on the concept, I think.

I had to add build dependencies on python-devel and python-setuptools to get
this to build.

The tarball I get from the upstream website is not the same as the tarball in
this package.  Did upstream change it over the past couple of hours or did you
somehow modify the tarball in the src.rpm?

Comment 7 Rob Crittenden 2008-01-17 14:22:37 UTC
The upstream tarball has changed. I first packaged this up last November and
just used the tar.gz I had downloaded then. I was unaware that upstream had
changed it.

That isn't a bad thing though. The change was the addition of a LICENSE.txt and
a setup.py. So I've dumped my setup.py and used upstreams and added the license
to %doc.

I also added the BuildRequires.

Spec URL: http://rcritten.fedorapeople.org/python-tgexpandingformwidget.spec
SRPM URL:
http://rcritten.fedorapeople.org/python-tgexpandingformwidget-0.1.3-4.fc7.src.rpm

Comment 8 Jason Tibbitts 2008-01-17 22:00:33 UTC
I think you can rremove the pyver macro from the top of the spec, as it seems to
be unused.

Shouldn't this have a dependency on TurboGears in some fashion?  rpmbuild
doesn't really generate proper dependencies for python code, so you have to do
it manually.  On the other hand, maybe it doesn't need TG; I'm honestly not an
expert, so I'll leave it to you.

Please use either %{buildroot} or $RPM_BUILD_ROOT but not both in the same specfile.

That's an easy fix, so I'll approve this package and you can fix it when you
check in.

Checklist:
* source files match upstream:
   5faa544f0ec0ea914b4b4719b3a8f54a1be732bf302b7ae918715f460c621b63  
   TGExpandingFormWidget-0.1.3.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named and is cleanly written
X specfile does not use macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* rpmlint is silent.
? final provides and requires (maybe needs turbogears)
   python-tgexpandingformwidget = 0.1.3-4.fc9
  =
   python(abi) = 2.5

* %check is not present; no test suite upstream.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

APPROVED; just s/\$RPM_BUILD_ROOT/%{buildroot}/ when you check in.

Comment 9 Rob Crittenden 2008-01-17 22:08:08 UTC
Ok, I'll fix those things up when I commit it.

New Package CVS Request
=======================
Package Name: python-tgexpandingformwidget
Short Description: A repeating form widget for TurboGears
Owners: rcritten
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

Comment 10 Kevin Fenzi 2008-01-18 03:36:27 UTC
cvs done.


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