https://bugzilla.redhat.com/show_bug.cgi?id=1403314 Stephen Gallagher <sgallagh@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(sgallagh@redhat.c | |om) | --- Comment #2 from Stephen Gallagher <sgallagh@xxxxxxxxxx> --- (In reply to Merlin Mathesius from comment #1) > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > ===== Questions ===== > > * Why no python3 version of this package? The upstream source appears to be > compatible. > Python 3 was failing to build, so I didn't bother investigating it. Today I took another look with some help from Igor Gnatenko and realized that it supports Python 3, but there was a bug that I could work around by setting the locale explicitly. The next version will support Python 3. > * Is PyPI the appropriate upstream source, rather than a git or other repo? > Yes, the upstream maintainer doesn't provide any other website for it. That page includes a link to the git repo. > * The hashs in the Source URL are not very pretty. Should the URL be > something like > > https://files.pythonhosted.org/packages/source/e/publicsuffix/publicsuffix-1. > 1.0.tar.gz > as recommended by > > https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/ > SourceURL#Python_Packages_.28pypi.29 ? > Unfortunately, that URL format doesn’t seem to work, but > https://pypi.io/packages/source/p/publicsuffix/publicsuffix-1.1.0.tar.gz > does. Thanks, I was just using the URL format I got from the download page. This is much cleaner. > > * Requires: publicsuffix-list? Why not just Recommends, since the list can > come from another source? The module even provides a method for retrieving > the most current list from publicsuffix.org. > Well, first of all it's tiny (the entire package contains a single text file). The reason for it is that if the user is running in a restricted network where they can't retrieve the updated file, we want to ensure they at least have the latest version provided by RPM. > * The source tarball includes bundled egg-info. Should that be removed > by adding “rm -rf %{srcname}.egg-info” after %autosetup? > Yes, that was an oversight. I thought I had done that... > * What is the reasoning for the .6 suffix on the Release: 0%{?dist}.6 ? > Well, I did it wrong, it should have been 0.6%{?dist}, but it's just a personal convention. I always reserve 0.x for pre-release work while I'm developing. I bump to release 1 when it's expected to go into a release. The .6 was the sixth discrete version of the RPM I had come up with. > ===== Issues ===== > > * %changelog initial entry needs to include a version-revision tag. > Thanks, fixed. > * License: field says MIT, but public_suffix_list.dat is MPLv2.0 and is > included in the source tarball and installed to > %{python2_sitelib}/publicsuffix/public_suffix_list.dat > when not replaced by a symlink for non-Fedora builds in the %install > section. You are correct. I changed the License: field to read "MIT and MPLv2.0" > > * subpackage corresponding to current python runtime must supply Provides: > python-publicsuffix > Fixed. Spec: https://sgallagh.fedorapeople.org/packagereview/python-publicsuffix/python-publicsuffix.spec SRPM: https://sgallagh.fedorapeople.org/packagereview/python-publicsuffix/python-publicsuffix-1.1.0-0.7.fc25.src.rpm COPR: https://copr.fedorainfracloud.org/coprs/sgallagh/python-publicsuffix/build/495902/ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx