https://bugzilla.redhat.com/show_bug.cgi?id=2210561 Maxwell G <maxwell@xxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value Blocks| |177841 (FE-NEEDSPONSOR) CC| |maxwell@xxxxxxx --- Comment #1 from Maxwell G <maxwell@xxxxxxx> --- Congratulations on your first package submission! Here are some comments. General comments ----------------------- Upstream's pyproject.toml has ``` [build-system] requires = ["poetry>=1.3"] build-backend = "poetry.masonry.api" ``` I'd use poetry-core as the build backend: ``` [build-system] requires = ["poetry-core>=1.3"] build-backend = "poetry.core.masonry.api" ``` ---- The maximum version pins in ``` [tool.poetry.dependencies] python = ">=3.9,<3.12" prettytable = ">=0.7.2,<3.8" ``` are also a problem for Fedora integration. You should relax them unless you're absolutely sure a certain version won't work. We only have one version of everything, so packages cannot be so picky or the package will become uninstallable when a dependency is updated or block the dependency update all together. Pinning Python versions are especially problematic; the Fedora Python maintainers are already preparing for the Python 3.12 rebuild. Specfile comments --------------------------- > Group: Applications/Engineering The Group tag should not be used in Fedora. ---- > BuildRequires: python3dist(poetry) > BuildRequires: python3dist(tox-current-env) Please remove these. %generate_buildrequires automatically handles these BuildRequires. ---- You should evenly space specfile sections to make the file more readable. There's two newlines between %prep and %generate_buildrequires, so there should also be two newlines between %build and %install and %install and %check and so on. Two newlines between specfile sections is a general convention. ---- You run `%pyproject_buildrequires -t` and `%tox`, but you don't actually have any tox envs configured in tox.ini. You should either properly configure tox to run your unit tests or entirely remove those invocations. ---- ``` # Fix command name in manpage ln -s hstsparser.py hstsparser # This runs outside the venv, so --version will return the default help2man -N ./hstsparser -o %{buildroot}%{_mandir}/man1/hstsparser.1 --version-string='%{version}' rm -f hstsparser ``` Building of manpages should happen in %build not %install. You can output the file to ./hstsparser.1 in the current directory during %build and then copy that file to %{buildroot}%{_mandir}/man1 during %install. Also, I'm not sure what ``` # This runs outside the venv, so --version will return the default ``` means. Fedora RPM builds do not use venvs. Maxwell's Python Review Checklist ----------------------- Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The license text is included and marked with %license. - [x] The package builds successfully in mock. - [x] The package is installable (checked by fedora-review). - [x] There are no relevant rpmlint errors. - [x] The package runs tests in %check. The package runs %pyproject_check_import and %tox. Running only %pyproject_check_import is enough to satisfy this guideline, but %tox shouldn't be run if it's NOOP. See the note above. - [x] The latest version is packaged or packaging an earlier version is justified. - [-] Libraries: The package name has a `python3-` prefix and uses the canonical project name - [x] Applications: The package follows the general Naming Guidelines for applications - [x] The pyproject macros are used. - [x] There are no bundled libraries. Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- 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=2210561 _______________________________________________ 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, report it: https://pagure.io/fedora-infrastructure/new_issue
