https://bugzilla.redhat.com/show_bug.cgi?id=1201338 Murilo Opsfelder Araujo <mopsfelder@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mopsfelder@xxxxxxxxx --- Comment #2 from Murilo Opsfelder Araujo <mopsfelder@xxxxxxxxx> --- Hello, Michael. Thanks a lot for your time on reviewing this package. I did update the package and would really appreciate if you could take a look at it again. (In reply to Michael Schwendt (Fedora Packager Sponsors Group) from comment #1) > [not a full review - just some drive-by comments] > > * https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text > * https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > * https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > I fixed file permissions in %files section. I also removed Buildroot directive. As to %license, I asked upstream maintainer to add a LICENSE file with a copy of the MIT license. > > > Version: 0.7.1 > > The included README says 0.7.0, so in case you've packaged a pre-release > snapshot, it should either follow the pre-release versioning guidelines when > keeping "Version: 0.7.1" or switch to "Version: 0.7.0" and follow the > post-release guidelines: > > * https://fedoraproject.org/wiki/Packaging:Guidelines#Version_and_Release > This time, I packaged a released version, i.e. 0.7.0. > > > %files > > %{python2_sitelib}/*egg-info > > %{python2_sitelib}/uniseg > > %defattr(0755,root,root,-) > > %{_bindir}/uniseg-dbpath > > %{python2_sitelib}/uniseg/wraptest.py > > %{python2_sitelib}/uniseg/samples/uniwrap.py > > %{python2_sitelib}/uniseg/samples/unibreak.py > > %{python2_sitelib}/uniseg/samples/wxwrapdemo.py > > %{python2_sitelib}/uniseg/sentencebreaktest.py > > %{python2_sitelib}/uniseg/graphemeclustertest.py > > %{python2_sitelib}/uniseg/wordbreaktest.py > > This %files section looks much as if it is the result of trial-and-error > build attempts: > > > %{python2_sitelib}/uniseg > > ... > > %{python2_sitelib}/uniseg/wraptest.py > > If %{python2_sitelib}/uniseg is a directory, the line > > %{python2_sitelib}/uniseg > > includes the directory and everything (anything!) in it. That is, all the > following lines lines in the %files section, which specify individual .py > files, are redundant. You also get warnings by rpmbuild: "warning: File > listed twice": > https://kojipkgs.fedoraproject.org//work/tasks/20/9210020/build.log > > Many packagers add a trailing slash to directory entries in %files sections > to make them more explicit, more readable, e.g. > > %{python2_sitelib}/uniseg/ > > does the same thing as > > %{python2_sitelib}/uniseg > > but makes it more clear to the reader that it is supposed to be a directory. > > https://fedoraproject.org/wiki/Packaging: > Guidelines#File_and_Directory_Ownership > https://fedoraproject.org/wiki/Packaging:UnownedDirectories > > The tool "rpmls" and "rpm" (rpm -qlvp …) are your friends when examining > package contentes / files lists. > I listed %{python2_sitelib}/uniseg/ in %files for readability. > > > %if 0%{?with_python3} > > %files -n python3-uniseg > > %{python3_sitelib}/*egg-info > > %{python3_sitelib}/uniseg > > %defattr(0755,root,root,-) > > %{_bindir}/uniseg-dbpath > > %{python3_sitelib}/uniseg/wraptest.py > > %{python3_sitelib}/uniseg/samples/uniwrap.py > > %{python3_sitelib}/uniseg/samples/unibreak.py > > %{python3_sitelib}/uniseg/samples/wxwrapdemo.py > > %{python3_sitelib}/uniseg/sentencebreaktest.py > > %{python3_sitelib}/uniseg/graphemeclustertest.py > > %{python3_sitelib}/uniseg/wordbreaktest.py > > %endif # with_python3 > > Same here. > Fixed that too. > > > # strip python3 from shebang line > > sed -i'' -e 's,^.*#!/usr/bin/python3.*$,#!/usr/bin/python,g' %{buildroot}%{_bindir}/uniseg-dbpath > > The file is included in both packages, so in package python3-uniseg > /usr/bin/uniseg-dbpath would execute via /usr/bin/python which may be Python > 2. > Since setup.py is called twice with different python interpreters (one with python2 and another with python3), the last installed /usr/bin/uniseg-dbpath file ends up having /usr/bin/python3 in the shebang line. > > > BuildArch: noarch > > BuildRequires: sqlite > > > Requires: sqlite > > Does it really need "sqlite" (no specific arch) at run-time? > > $ grep sqlite * -R > Binary file uniseg/__pycache__/db.cpython-34.pyo matches > Binary file uniseg/__pycache__/db.cpython-34.pyc matches > Binary file uniseg/ucd.sqlite3 matches > uniseg/db.py:import sqlite3 > uniseg/db.py: dbname = 'ucd.sqlite3' > uniseg/db.py: _conn = sqlite3.connect(_dbpath) > > Typically, a comment should explain such explicit Requires: > https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires > I removed sqlite from Requires. > [...] > > Consider pointing the fedora-review tool at review tickets like this: > fedora-review -b 1201338 Spec URL: https://mopsfelder.fedorapeople.org/SPECS/python-uniseg.spec SRPM URL: https://mopsfelder.fedorapeople.org/SRPMS/python-uniseg-0.7.0-1.fc23.src.rpm Koji task: http://koji.fedoraproject.org/koji/taskinfo?taskID=9582619 -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review