Bug 427275 - Review Request: python-tgexpandingformwidget - A repeating form widget for TurboGears
Review Request: python-tgexpandingformwidget - A repeating form widget for T...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-01-02 13:38 EST by Rob Crittenden
Modified: 2008-01-18 10:39 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-01-18 10:39:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Rob Crittenden 2008-01-02 13:38:06 EST
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 12:20:27 EST
A couple of notes:

The URL in the URL: tag doesn't exist;  I'd suggest

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 13:20:58 EST
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 18:19:16 EST
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-16 22:11:06 EST
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-16 23:40:57 EST
Wrote a setup.py that builds the package from the tar.gz.

Spec URL: http://rcritten.fedorapeople.org/python-tgexpandingformwidget.spec
Comment 6 Jason Tibbitts 2008-01-17 02:32:58 EST
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 09:22:37 EST
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
Comment 8 Jason Tibbitts 2008-01-17 17:00:33 EST
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.

* source files match upstream:
* 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 17:08:08 EST
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
Cvsextras Commits: yes
Comment 10 Kevin Fenzi 2008-01-17 22:36:27 EST
cvs done.

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