https://bugzilla.redhat.com/show_bug.cgi?id=2008657 david08741@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(david08741@gmail. | |com) | --- Comment #2 from david08741@xxxxxxxxx --- Spec URL: https://davidsch.fedorapeople.org/v1/python-gelidum.spec SRPM URL: https://davidsch.fedorapeople.org/v1/python-gelidum-0.5.3-1.fc36.src.rpm (In reply to Ben Beasley from comment #1) > ===== Issues ===== > > - To get the runtime requirements in order to run tests, i.e. > install_requires, > you should change > > %pyproject_buildrequires > > to > > %pyproject_buildrequires -r > > The only reason it’s working for you is that there are currently no such > requirements. Unfortunately that doesn't work because the project doesn't provide a project.toml or setup.cfg file. I did assume that was a limitation of the macros. > - There is a comment left over from another package: > > # For python3-pello, %%{pyproject_files} handles code files, but > # executables, documentation and license must be listed in the spec file: Thanks, fixed. > > - Over half of the installed RPM size is a copy of the test suite. I think > installing it is allowable, but I’m not convinced it is valuable. Consider > (at your discretion): > > %exclude %{python3_sitelib}/gelidum/tests As per https://github.com/rpm-software-management/rpm/issues/1094 that does not seem to be the proper usage of that macro, but if I remember correctly from the devel mailing list discussion no better alternative for such a use case was suggested. An extra -test subpackage could be used, though. > > - You can, if you like, drop “%license LICENSE” since pyproject-rpm-macros > properly tags the LICENSE file in the dist-info directory. It’s good to > verify this with “rpm -qL -p …”, since there are some Python projects where > this doesn’t work. I would prefer having it explicitly listed. Otherwise an update might break it, but good to know that it also works automatically :-) > - The spec file is missing a changelog with an initial entry. See > https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs for > the > traditional way of keeping a changelog. > > If you want to opt in to rpmautospec > (https://docs.pagure.org/Fedora-Infra.rpmautospec/), you can instead change > > Release: 1%{?dist} > > to > > Release: %autorelease > > and append to the end of the spec file: > > %changelog > %autochangelog > Thanks, yes I did indeed miss the autochangelog entry. > - While I think the URL referencing an upstream PR is technically adequate > justification for 17.patch, it would be helpful to add a spec file comment > with a quick summary, like: > > # Python 3.10 support I changed the file name to python-3.10.patch > > - The Summary field should not end with a period. > > python3-gelidum.noarch: W: summary-ended-with-dot C Freeze your objects > in python. > Fixed Thanks for the review 👍 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2008657 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
