https://bugzilla.redhat.com/show_bug.cgi?id=2095005 --- Comment #4 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- Thank you for the detailed review! ----- > As we discussed in #fedora-python, this should be removed and patched out of the setup.py if the later is necessary. This is probably worthwhile to bring up upstream. Upstream issue filed: Consider dropping the pytest-runner test dependency https://github.com/naimetti/rfc3339-validator/issues/6 Upstream PRs filed: Drop the pytest-runner test dependency and “setup.py test” support https://github.com/naimetti/rfc3339-validator/pull/7 Include files for tox testing in the sdist https://github.com/naimetti/rfc3339-validator/pull/8 The spec file is modified with a patch derived from upstream PR#7, and the manual BR on python3dist(pytest-runner) is removed. As you noted, the tox configuration is more trouble that it is worth; it is easy enough to unpin versions with sed, but most of the dependencies in requirements_dev.txt are things like coverage, flake8, etc. that I would have to patch back out. Therefore the spec file continues to rely on manual BR’s for testing and invokes the tests with the %pytest macro rather than %tox. ----- > Also, I prefer the %{py3_dist X} dependency format over python3dist. It's less typing and it respects the value of %python3_pkgversion, which occasionally needs to be changed, such as when packaging for alternative Python stacks in EPEL. It also has some logic to ensure that canonical project names are used[0]. Feel free to leave it as is if you'd like. I appreciate how the %{py3_dist …} macro helps form canonical project names, which makes it easier to do manual BR’s. On the other hand, it seems to be confusing to some packagers, and I have even seen people asking for %{py3_dist …} to be switched to python3dist(…) in package reviews. I am grateful for the information on support for %{python3_pkgversion}, in case that is ever something other than “3” on an EPEL newer than EPEL7. That’s good to know. Still, in the absence of an official Fedora-wide preferred form or obvious community consensus, I’ll opt to keep the python3dist(…) form here for consistency with my other packages. ----- > If you're trying to escape the newline here, this doesn't work as expected. You know, I picked up that habit a long time ago after I saw it somewhere, and I could have sworn I tried it. But you’re right, so I’ve fixed it, and I’ll make a project of fixing it in other packages that use the same pattern. At least the extra newline is only a trivial annoyance rather than a serious defect. ----- > It looks like the unit tests are being installed to `%{python3_sitelib}`. Is that something we want? This doesn’t seem to be the case. Check files.dir in the fedora-review output, or: > $ rpm -q -p --filesbypkg /var/lib/mock/fedora-rawhide-x86_64/result/python3-rfc3339-validator-0.1.4-1.fc37.noarch.rpm > python3-rfc3339-validator /usr/lib/python3.11/site-packages/__pycache__/rfc3339_validator.cpython-311.opt-1.pyc > python3-rfc3339-validator /usr/lib/python3.11/site-packages/__pycache__/rfc3339_validator.cpython-311.pyc > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/AUTHORS.rst > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/INSTALLER > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/LICENSE > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/METADATA > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/WHEEL > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator-0.1.4.dist-info/top_level.txt > python3-rfc3339-validator /usr/lib/python3.11/site-packages/rfc3339_validator.py > python3-rfc3339-validator /usr/share/doc/python3-rfc3339-validator > python3-rfc3339-validator /usr/share/doc/python3-rfc3339-validator/AUTHORS.rst > python3-rfc3339-validator /usr/share/doc/python3-rfc3339-validator/CONTRIBUTING.rst > python3-rfc3339-validator /usr/share/doc/python3-rfc3339-validator/HISTORY.rst > python3-rfc3339-validator /usr/share/doc/python3-rfc3339-validator/README.md ----- New Spec URL: https://music.fedorapeople.org/20220629/python-rfc3339-validator.spec New SRPM URL: https://music.fedorapeople.org/20220629/python-rfc3339-validator-0.1.4-1.fc36.src.rpm -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2095005 _______________________________________________ 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
