Bug 980399 - Review Request: python-ntplib - Python module that offers a simple interface to query NTP servers
Review Request: python-ntplib - Python module that offers a simple interface ...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Robert Kuska
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-02 05:28 EDT by Vratislav Podzimek
Modified: 2016-04-18 06:16 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-07-10 07:26:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rkuska: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Vratislav Podzimek 2013-07-02 05:28:00 EDT
Spec URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib.spec
SRPM URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib-0.3.0-1.fc19.src.rpm

Description:
The ntplib is a python module that offers a simple interface to query NTP
servers. It also provides utility functions to translate NTP fields' values to
text (mode, leap indicator...). Since it's pure Python, and only depends on core
modules, it should work on any platform with a Python implementation. (Note: this would be a useful package for the Anaconda installer)

Fedora Account System Username: vpodzime
Comment 1 Vratislav Podzimek 2013-07-02 05:35:07 EDT
One more note -- the 'ntplib' Python package [1] uses the LGPL licence which is not listed under the Good licences for the Fedora [2], but the "GPL+" is described as GPL or LGPL without version specified.

[1] https://pypi.python.org/pypi/ntplib/
[2] https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses
Comment 2 Mario Blättermann 2013-07-02 13:34:01 EDT
$ licensecheck -r *
ntplib.py: LGPL (v2 or later) (with incorrect FSF address)
setup.py: *No copyright* UNKNOWN
test_ntplib.py: *No copyright* UNKNOWN

The file ntplib.py is that file which we refer to as the software we want to package. Don't bother with ambiguous license files. The header in ntplib.py and PKGINFO say LGPL or later versions, so the license tag is LGPLv2+. Moreover, the CHANGELOG contains this:

version 0.1.8 - 2010-02-20
- change to LGPL license
- cleanup


Is it possible to run test_ntplib.py in a %check section? Would this make sense?


rm -rf $RPM_BUILD_ROOT
is an artifact from older Fedora releases. Don't know why rpmdev-newspec still adds it to a spec file. You can safely drop that line.

You have to add CHANGELOG and COPYING.LESSER to %doc.

BTW, the incorrect FSF address in ntplib.py is worth to be reported upstream.

One of the description lines is too long, line 16 has 81 characters. Should be no more than 80.
Comment 3 Robert Kuska 2013-07-03 03:34:16 EDT
I'll take this for a review.
Comment 4 Vratislav Podzimek 2013-07-03 05:13:40 EDT
(In reply to Mario Blättermann from comment #2)
> $ licensecheck -r *
> ntplib.py: LGPL (v2 or later) (with incorrect FSF address)
> setup.py: *No copyright* UNKNOWN
> test_ntplib.py: *No copyright* UNKNOWN
> 
> The file ntplib.py is that file which we refer to as the software we want to
> package. Don't bother with ambiguous license files. The header in ntplib.py
> and PKGINFO say LGPL or later versions, so the license tag is LGPLv2+.
> Moreover, the CHANGELOG contains this:
> 
> version 0.1.8 - 2010-02-20
> - change to LGPL license
> - cleanup
I'm changing the tag to LGPLv2+.

> Is it possible to run test_ntplib.py in a %check section? Would this make
> sense?
I was thinking about it, but those tests need network connection and try to poke some NTP servers. They fail e.g. on the networks with blocked NTP traffic to external servers.

> 
> rm -rf $RPM_BUILD_ROOT
> is an artifact from older Fedora releases. Don't know why rpmdev-newspec
> still adds it to a spec file. You can safely drop that line.
Dropped.

> 
> You have to add CHANGELOG and COPYING.LESSER to %doc.
Hmm, that would mean some further changes as they are not installed by the setup.py anywhere.

> 
> BTW, the incorrect FSF address in ntplib.py is worth to be reported upstream.
Patch sent to the upstream maintainer.

> 
> One of the description lines is too long, line 16 has 81 characters. Should
> be no more than 80.
Fixed.

Thanks!
Comment 5 Mario Blättermann 2013-07-03 07:22:06 EDT
(In reply to Vratislav Podzimek from comment #4)
> (In reply to Mario Blättermann from comment #2)
> > You have to add CHANGELOG and COPYING.LESSER to %doc.
> Hmm, that would mean some further changes as they are not installed by the
> setup.py anywhere.
> 
What further changes you mean? Just write

%doc CHANGELOG COPYING.LESSER

and rpm will do the rest. In almost all cases, such files are not installed anywhere by default. Not only in Python setup scripts, also in other software.
Comment 6 Vratislav Podzimek 2013-07-03 08:41:16 EDT
(In reply to Mario Blättermann from comment #5)
> (In reply to Vratislav Podzimek from comment #4)
> > (In reply to Mario Blättermann from comment #2)
> > > You have to add CHANGELOG and COPYING.LESSER to %doc.
> > Hmm, that would mean some further changes as they are not installed by the
> > setup.py anywhere.
> > 
> What further changes you mean? Just write
> 
> %doc CHANGELOG COPYING.LESSER
> 
> and rpm will do the rest. In almost all cases, such files are not installed
> anywhere by default. Not only in Python setup scripts, also in other
> software.
Great, thanks! I didn't know %doc works that way. I've updated the .spec file.
Comment 7 Vratislav Podzimek 2013-07-04 03:57:13 EDT
(In reply to Vratislav Podzimek from comment #4)
> > 
> > BTW, the incorrect FSF address in ntplib.py is worth to be reported upstream.
> Patch sent to the upstream maintainer.
Patch applied upstream, I've created a new version of the .spec and SRPM.

Spec URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib.spec
SRPM URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib-0.3.1-1.fc19.src.rpm
Comment 8 Robert Kuska 2013-07-04 04:00:15 EDT
Add please BuildRequires:  python-setuptools.
Comment 9 Vratislav Podzimek 2013-07-05 04:45:39 EDT
(In reply to Robert Kuska from comment #8)
> Add please BuildRequires:  python-setuptools.
Done.

Spec URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib.spec
SRPM URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib-0.3.1-1.fc20.src.rpm
Comment 10 Robert Kuska 2013-07-08 02:40:05 EDT
(In reply to Vratislav Podzimek from comment #9)
> (In reply to Robert Kuska from comment #8)
> > Add please BuildRequires:  python-setuptools.
> Done.
> 
> Spec URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib.spec
> SRPM URL:
> http://vpodzime.fedorapeople.org/ntplib/python-ntplib-0.3.1-1.fc20.src.rpm

Checking: python-ntplib-0.3.1-1.fc18.noarch.rpm
python-ntplib.noarch: W: incoherent-version-in-changelog 0.3.0-1 ['0.3.1-1.fc18', '0.3.1-1']
1 packages and 0 specfiles checked; 0 errors, 1 warnings.



You forgot to add new (or fix old) changelog entry after updating source.

Otherwise it looks fine.
Comment 11 Vratislav Podzimek 2013-07-08 05:38:13 EDT
(In reply to Robert Kuska from comment #10)
> (In reply to Vratislav Podzimek from comment #9)
> > (In reply to Robert Kuska from comment #8)
> > > Add please BuildRequires:  python-setuptools.
> > Done.
> > 
> > Spec URL: http://vpodzime.fedorapeople.org/ntplib/python-ntplib.spec
> > SRPM URL:
> > http://vpodzime.fedorapeople.org/ntplib/python-ntplib-0.3.1-1.fc20.src.rpm
> 
> Checking: python-ntplib-0.3.1-1.fc18.noarch.rpm
> python-ntplib.noarch: W: incoherent-version-in-changelog 0.3.0-1
> ['0.3.1-1.fc18', '0.3.1-1']
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
> 
> 
> You forgot to add new (or fix old) changelog entry after updating source.
Fixed, thanks.
Comment 12 Robert Kuska 2013-07-08 05:53:35 EDT
Nice, looks good now. I approve this package.
Comment 13 Vratislav Podzimek 2013-07-09 09:16:14 EDT
New Package SCM Request
=======================
Package Name: python-ntplib
Short Description: Python module that offers a simple interface to query NTP servers
Owners: vpodzime
Branches: f19
InitialCC:
Comment 14 Gwyn Ciesla 2013-07-09 09:23:38 EDT
Git done (by process-git-requests).
Comment 15 Fedora Update System 2013-07-10 07:23:23 EDT
python-ntplib-0.3.1-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/python-ntplib-0.3.1-1.fc19
Comment 16 Fedora Update System 2013-07-22 21:15:55 EDT
python-ntplib-0.3.1-1.fc19 has been pushed to the Fedora 19 stable repository.

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