Bug 1156619 - Review Request: python-tzlocal - tzinfo object for the local timezone
Summary: Review Request: python-tzlocal - tzinfo object for the local timezone
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-10-24 19:38 UTC by Piotr Popieluch
Modified: 2014-12-11 06:32 UTC (History)
2 users (show)

Fixed In Version: python-tzlocal-1.1.2-4.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-10-27 21:00:42 UTC
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Comment 1 Parag AN(पराग) 2014-10-25 08:41:55 UTC
1) rpmlint on all generated rpms gave following common messages
W: summary-not-capitalized C tzinfo object for the local timezone
E: description-line-too-long C This Python module returns a tzinfo object with the local timezone information under Unix and Win-32.
=> you may want to add some word or re-word the summary
   also limit the line to 80 characters in description

2) I see some removals are done in %install section. Can you add comments to explain why you need to execute rm commands there?

3) you should in %prep
rm -rf *.egg-info

Comment 2 Piotr Popieluch 2014-10-25 11:10:12 UTC
Thank you for commenting.

I have added the rm commands in %install so that unit tests are not installed. I could not find a policy which mandates this but thought that it would be clean to not install those files as users will not use them. I got the idea from the python-setuptools.spec . If you think they need to be installed I will delete the rm lines. I've added comments to the spec to explain this.


I have updated the spec and srpm:

- deleted group tag
- added license to python3 module
- rewritten summary
- wrapped description
- added rm -rf *.egg-info to %prep
- added comments to the rm commands in %install section

Comment 3 Parag AN(पराग) 2014-10-25 11:40:15 UTC
I can't see any updated package. Please be habitual with bumping release number. Generating new package and posting links of new SPEC and SRPM.

We generally review SRPM which includes SPEC and not just SPEC.

Comment 4 Parag AN(पराग) 2014-10-25 11:41:49 UTC
Don't forget to add changes information in a new Changelog entry.

Comment 5 Piotr Popieluch 2014-10-25 11:51:24 UTC
bumped release number:

Spec URL: http://vps533.directvps.nl/python-tzlocal.spec
SRPM URL: http://vps533.directvps.nl/python-tzlocal-1.1.2-2.fc20.src.rpm

Comment 6 Parag AN(पराग) 2014-10-25 13:07:32 UTC
This looks good now :)

Review:
+ is OK
- is Needs Work

+ Package built successful in mock (f22 x86_64)

- rpmlint on generated rpms gave output
python-tzlocal.src: W: spelling-error Summary(en_US) tzinfo -> tinfoil
python-tzlocal.src: W: spelling-error %description -l en_US tzinfo -> tinfoil
python3-tzlocal.noarch: W: spelling-error Summary(en_US) tzinfo -> tinfoil
python3-tzlocal.noarch: W: summary-not-capitalized C tzinfo object for the local timezone
python3-tzlocal.noarch: W: spelling-error %description -l en_US tzinfo -> tinfoil
python-tzlocal.noarch: W: spelling-error Summary(en_US) tzinfo -> tinfoil
python-tzlocal.noarch: W: spelling-error %description -l en_US tzinfo -> tinfoil
3 packages and 0 specfiles checked; 0 errors, 7 warnings.

+ Source verified with upstream as sha256sum
srpm tarball: 4d9ddb8d5eab086e3a7c504c6e994ffa85df43e40da4d6be776218be051c677a
upstream tarball: 4d9ddb8d5eab086e3a7c504c6e994ffa85df43e40da4d6be776218be051c677a

+ License is CCO and included in LICENSE.txt file

+ follow python packaging guidelines

+ rest also looks as per packaging guidelines.

Suggestion:
1) Summary for main package python-tzlocal should be "A Python module that tries to figure out what your local timezone is". This is a nice summary as can be seen on github page

2) %description should contain some more information like

This Python module returns a tzinfo object with the local timezone information. It requires pytz, and returns pytz tzinfo objects.

This module attempts to fix a glaring hole in pytz, that there is no way to get the local timezone information, unless you know the zoneinfo name.

3) use above for python3-tzlocal %description


Fix above and for this if you want you don't need to bump release number and package will be ready to be approved.

Comment 7 Parag AN(पराग) 2014-10-26 02:18:26 UTC
Please provide updated SPEC and SRPM links here so I can approve this package.

Comment 8 Piotr Popieluch 2014-10-26 08:48:22 UTC
Updated SPEC + SRPM with new summary/description as suggested:

Spec URL: http://vps533.directvps.nl/python-tzlocal.spec
SRPM URL: http://vps533.directvps.nl/python-tzlocal-1.1.2-2.fc20.src.rpm

Comment 9 Parag AN(पराग) 2014-10-26 11:42:33 UTC
APPROVED this package.

Comment 10 Parag AN(पराग) 2014-10-26 11:44:21 UTC
and yes have the same summary for python-tzlocal and python3-tzlocal otherwise rpmlint will complain warning :-)

Comment 11 Piotr Popieluch 2014-10-26 12:39:54 UTC
missed that one.. fixed, thanks.

Comment 12 Piotr Popieluch 2014-10-26 13:05:14 UTC
New Package SCM Request
=======================
Package Name: python-tzlocal
Short Description: A Python module that tries to figure out what your local timezone is
Upstream URL: https://github.com/regebro/tzlocal
Owners: piotrp
Branches: f19 f20 f21
InitialCC: piotrp

Comment 13 Gwyn Ciesla 2014-10-27 12:23:07 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2014-10-27 20:16:03 UTC
python-tzlocal-1.1.2-2.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-2.fc21

Comment 15 Fedora Update System 2014-10-27 20:22:11 UTC
python-tzlocal-1.1.2-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-2.fc20

Comment 16 Fedora Update System 2014-10-27 20:23:00 UTC
python-tzlocal-1.1.2-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-2.fc19

Comment 17 Fedora Update System 2014-10-28 06:52:04 UTC
python-tzlocal-1.1.2-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-3.fc20

Comment 18 Fedora Update System 2014-10-28 06:52:34 UTC
python-tzlocal-1.1.2-3.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-3.fc21

Comment 19 Fedora Update System 2014-10-28 06:55:18 UTC
python-tzlocal-1.1.2-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-3.fc19

Comment 20 Fedora Update System 2014-11-07 02:31:20 UTC
python-tzlocal-1.1.2-3.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2014-11-07 02:35:47 UTC
python-tzlocal-1.1.2-3.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2014-11-10 05:59:02 UTC
python-tzlocal-1.1.2-3.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Piotr Popieluch 2014-11-22 10:40:49 UTC
Package Change Request
======================
Package Name: python-tzlocal
New Branches: el6 epel7
Owners: piotrp

Comment 24 Gwyn Ciesla 2014-11-24 13:41:35 UTC
Git done (by process-git-requests).

Comment 25 Fedora Update System 2014-11-24 21:06:28 UTC
python-tzlocal-1.1.2-4.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-4.el7

Comment 26 Fedora Update System 2014-11-24 21:08:55 UTC
python-tzlocal-1.1.2-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/python-tzlocal-1.1.2-4.el6

Comment 27 Fedora Update System 2014-12-11 06:27:26 UTC
python-tzlocal-1.1.2-4.el7 has been pushed to the Fedora EPEL 7 stable repository.

Comment 28 Fedora Update System 2014-12-11 06:32:10 UTC
python-tzlocal-1.1.2-4.el6 has been pushed to the Fedora EPEL 6 stable repository.


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