Bug 895077 - Review Request: python-docopt - Pythonic argument parser, that will make you smile
Summary: Review Request: python-docopt - Pythonic argument parser, that will make you...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bohuslav "Slavek" Kabrda
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1384589 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-01-14 14:03 UTC by Martin Sivák
Modified: 2016-11-01 17:42 UTC (History)
7 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-03-13 06:58:18 UTC
Type: ---
Embargoed:
bkabrda: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Martin Sivák 2013-01-14 14:03:48 UTC
Spec URL: http://msivak.fedorapeople.org/newpkg/python-docopt.spec
SRPM URL: http://msivak.fedorapeople.org/newpkg/python-docopt-0.5.0-1.fc17.src.rpm
Description: Pythonic argument parser, that uses the help message to generate the parsing rules.
Fedora Account System Username: msivak

Comment 1 Bohuslav "Slavek" Kabrda 2013-01-14 14:37:18 UTC
I'll take this for a review.

Comment 2 Bohuslav "Slavek" Kabrda 2013-01-14 14:46:22 UTC
- python-setuptools-devel is no longer used, you should use just python-setuptools.
- If you want the packages to go just to Fedora, you can remove the macro definitions from the beginning, they are already defined in macrofiles.
- The line with "rm -rf ${buildroot}%{python_sitelib}/setuptools/tests" seems to be useless (at least in mock build, so it should be removed).
Otherwise the package seems to be ok, so once you fix these minorities, I'll approve.

Comment 3 Martin Sivák 2013-01-14 15:46:28 UTC
I updated the spec file according to the comment and reuploaded the files to the same urls.

Comment 4 Fabian Affolter 2013-01-14 21:28:27 UTC
Just some quick comments:

- %dos are missing (e.g. LICENSE-MIT, README.md)
- The %description and the %summary needs some tweaking. At the moment both are not very useful.
- There is no *.egg-info file in the upstream tarball. I would suggest to leave it in place. You never know what will happen in the future.
- There is an folder with examples. Perhaps it's worth to pack those files.

The 'review' flag should be set by the reviewer and not the reporter.

Comment 5 Bohuslav "Slavek" Kabrda 2013-01-15 06:50:38 UTC
(In reply to comment #4)
> Just some quick comments:
> 
> - %dos are missing (e.g. LICENSE-MIT, README.md)

They aren't in the upstream tarball, in which case the guidelines don't mandate them to be present. [1] says " If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc."

> - The %description and the %summary needs some tweaking. At the moment both
> are not very useful.

I disagree, from my point of view it says enough. Anyway, this is not a blocker for me, so I'll leave it up to Martin's best judgement whether he'll change this or not before importing to dist-git.

> - There is no *.egg-info file in the upstream tarball. I would suggest to
> leave it in place. You never know what will happen in the future.

There is a docopt.egg-info directory and I agree that it should be removed and regenerated during build. I consider this to be a good practice, that makes sure everything that can be regenerated is regenerated.

> - There is an folder with examples. Perhaps it's worth to pack those files.

If it was in the upstream tarball, I'd concur. This way, I'd maybe just encourage upstream to include it (and also include the license text), but this is not mandatory.

> 
> The 'review' flag should be set by the reviewer and not the reporter.

True :) And thanks for your comments!


Martin, please next time when doing changes during review, bump the release and add a proper changelog entry.


Since I see no blockers, this package is APPROVED. Martin, please consider improving the summary and description before importing to dist-git and encouraging upstream to include license text and examples in the next tarball.

[1] https://fedoraproject.org/wiki/Packaging:ReviewGuidelines?rd=Packaging/ReviewGuidelines#Things_To_Check_On_Review

Comment 6 Martin Sivák 2013-01-15 09:39:47 UTC
New Package SCM Request
=======================
Package Name: python-docopt
Short Description: Pythonic argument parser that uses the help message to generate the parsing rules.
Owners: msivak
Branches: f17 f18
InitialCC:

Comment 7 Gwyn Ciesla 2013-01-15 12:02:26 UTC
Git done (by process-git-requests).

Comment 8 Michal Minar 2013-08-22 14:04:38 UTC
Package Change Request
======================
Package Name: python-docopt
New Branches: el7
Owners: msivak miminar
InitialCC: 

python-docopt is being currently used in openlmi side project, which will later probably go to rhel7

Comment 9 Christopher Meng 2013-08-22 15:10:13 UTC
el7?

No such branch currently?

Comment 10 Gwyn Ciesla 2013-08-22 15:27:24 UTC
No such branch.

Comment 11 Martin Sivák 2015-02-20 10:58:00 UTC
Package Change Request
======================
Package Name: python-docopt
New Branches: el6
Owners: msivak miminar
InitialCC: 

There is a request for docopt on CentOS 6 - https://bugzilla.redhat.com/show_bug.cgi?id=1194598

I think it is not a bad idea.

Comment 12 Gwyn Ciesla 2015-02-20 13:54:03 UTC
Git done (by process-git-requests).

Comment 13 Athos Ribeiro 2016-11-01 17:42:05 UTC
*** Bug 1384589 has been marked as a duplicate of this bug. ***


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