Bugzilla will be upgraded to version 5.0. The upgrade date is tentatively scheduled for 2 December 2018, pending final testing and feedback.
Bug 614299 - Review Request: python-ordereddict - Py2.7's new collections.OrderedDict that works in Python 2.4-2.6.
Review Request: python-ordereddict - Py2.7's new collections.OrderedDict that...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Toshio Ernie Kuratomi
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 956422
  Show dependency treegraph
 
Reported: 2010-07-13 23:47 EDT by James Ni
Modified: 2013-04-25 11:41 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-01-24 03:52:44 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
a.badger: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description James Ni 2010-07-13 23:47:45 EDT
Spec URL: http://jamesni.fedorapeople.org/python-ordereddict/python-ordereddict.spec
SRPM URL: http://jamesni.fedorapeople.org/python-ordereddict/python-ordereddict-1.1-1.fc13.src.rpm

Description: Drop-in substitute for Py2.7's new collections.OrderedDict. The recipe has big-oh performance that matches regular dictionaries (amortized O(1) insertion/deletion/lookup and O(n) iteration/repr/copy/equality_testing).

Originally based on http://code.activestate.com/recipes/576693/
Comment 1 James Ni 2010-07-14 04:03:56 EDT
This is my first package, and I am seeking a sponsor.
Comment 2 Toshio Ernie Kuratomi 2010-10-27 19:19:16 EDT
If you could review someone else's package so that I have more to evaluate you on I'd appreciate it.  Here's a preliminary review:

NEEDSWORK:
* There is an upstream tarball for this at pypi:
  http://pypi.python.org/pypi/ordereddict
  - The tarball in the srpm does not match the tarball on pypi
* We should be sticking to 80 columns or less in spec files.  If the Summary is longer than 80 columns, chances are that it's too long.
* %define should be changed to %global as %define inside of %{} is only working due to an rpm bug.

You can reach me on IRC: abadger1999 on freenode.net, email toshio fedoraproject.org, or just CC me on a review request that you take.
Comment 3 Jens Petersen 2010-10-27 19:43:08 EDT
(James is already sponsored now as a packager.)
Comment 4 Toshio Ernie Kuratomi 2010-10-27 22:09:40 EDT
Good:
* Named according to Guidelines
* License is MIT in source file and spec
* Spec file is readable
* No locales to process
* No shared libraries to process
* No bundled libraries
* Not relocatable
* Package owns all directories that it creates
* Files are not listed twice
* Permissions set correctly
* Macros used consistently
* Code, not content
* No doc files affect runtime
* Not a GUI package
* Does not own files from other packages
* All filenames utf-8
* Builds in koji with %doc removal and upstream's tarball

Needswork:
* Source does not match upstream.   There is an upstream tarball for this at pypi:
  http://pypi.python.org/pypi/ordereddict
  - The tarball in the srpm does not match the tarball on pypi
* We should be sticking to 80 columns or less in spec files.  If the Summary is
longer than 80 columns, chances are that it's too long.
* %define should be changed to %global as %define inside of %{} is only working
due to an rpm bug.

rpmlint:
python-ordereddict.noarch: W: summary-ended-with-dot C A drop-in substitute for Py2.7's new collections.OrderedDict that works in Python 2.4-2.6.
python-ordereddict.noarch: E: summary-too-long C A drop-in substitute for Py2.7's new collections.OrderedDict that works in Python 2.4-2.6.

Don't need the period at the end, "A " at the beginning  Make it shorter.  Something like "Implementation of Python 2.7's OrderedDict"

python-ordereddict.noarch: W: spelling-error %description -l en_US lookup -> lockup, hookup, look up
python-ordereddict.noarch: W: spelling-error %description -l en_US repr -> rept, rep, repro

These are fine.  Both are spelled correctly in their domain.

python-ordereddict.noarch: E: description-line-too-long C Drop-in substitute for Py2.7's new collections.OrderedDict. The recipe has big-oh performance that matches regular dictionaries (amortized O(1) insertion/deletion/lookup and O(n) iteration/repr/copy/equality_testing).

Noted above.

python-ordereddict.noarch: W: no-documentation

If upstream provides no documentation we don't need to ship any

2 packages and 0 specfiles checked; 4 errors, 7 warnings.


Notes:
* Should query upstream to include license text
Comment 5 James Ni 2010-10-27 23:09:02 EDT
Thanks for you comment, i just update the spec file base on your comments and put it in my fedorapeople account. It pass the rpmlint without errors now. I also sent a requirement to upstream maintainer to ask him include the license text in the source. I have to wait his feedback.
Comment 6 Toshio Ernie Kuratomi 2010-10-27 23:40:29 EDT
When you make changes to the spec file, you need to increment the Release: field and add a changelog entry.

You actually don't need to wait on his feedback.  Shipping a license file is not a requirement as long as the license is clear from what *is* in the tarball.  There's only one code file in the tarball and that file has the MIT license text so it's clear what the status is.  You can update to upstream's tarball without issue.
Comment 7 James Ni 2010-10-28 03:28:58 EDT
Thanks, just increment the release field and add a changelog entry. Also update to using upstream's tarball.

Spec URL:
http://jamesni.fedorapeople.org/python-ordereddict/python-ordereddict.spec

SRPM URL:
http://jamesni.fedorapeople.org/python-ordereddict/python-ordereddict-1.1-2.fc13.src.rpm
Comment 8 Toshio Ernie Kuratomi 2010-10-28 04:36:30 EDT
remaining issues fixed.  APPROVED.

Note:   looks like upstream updated the tarball in place.  There's a new upstream 1.1 tarball with a different md5sum and the license file included.  I verified the tarball in the srpm matched the old tarball.  You might want to update to the new tarball before importing the package.
Comment 9 Toshio Ernie Kuratomi 2010-11-16 10:34:10 EST
Ping.  you can go ahead with the SCM Request and getting the package imported now.

I'd love to see this in EPEL5/6 as well as Fedora if you're interested in maintaining it there.
Comment 11 James Ni 2010-11-21 21:44:55 EST
New Package SCM Request
=======================
Package Name: python-ordereddict
Short Description: OrderedDict that works in Python 2.4-2.6
Owners: jamesni
Branches: f13 f14 el5 el6
InitialCC: jamesni
Comment 12 James Ni 2010-11-22 00:05:25 EST
New Package SCM Request
=======================
Package Name: python-ordereddict
Short Description: OrderedDict that works in Python 2.4-2.6
Owners: jamesni
Branches: f13 el5 el6
InitialCC: jamesni
Comment 13 Jason Tibbitts 2010-11-22 08:52:37 EST
Git done (by process-git-requests).
Comment 14 Toshio Ernie Kuratomi 2011-01-11 11:49:19 EST
I see that you've built package in koji for el5 and f13 but not submitted them to the download repositories.

https://koji.fedoraproject.org/koji/packageinfo?packageID=11231

(The el5 and f13 packages are tagged into *-candidate right now.)

You can submit them to the stable repos in bodhi:

https://admin.fedoraproject.org/updates
http://fedoraproject.org/wiki/Bodhi_Guide#Workflow

You can also close this bug as the review is complete.
Comment 15 James Ni 2011-01-24 03:52:44 EST
Hi, I have built package in koji and sumbit them to the stable repos. You can use yum to install it now. This bug will be closed.

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