https://bugzilla.redhat.com/show_bug.cgi?id=2114603 Miro Hrončok <mhroncok@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok@xxxxxxxxxx --- Comment #2 from Miro Hrončok <mhroncok@xxxxxxxxxx> --- Spec sanity check: > %{?!python3_pkgversion:%global python3_pkgversion 3} This should never be needed, why is it in the spec? > %global srcname kgb Setting and using this IMHO makes the spec file harder to read (I cannot for example copy-paste the URL to my web browser) with very little benefit. It's not like I can copy-paste the entire spec, change this macro, and ship that as another package, or is it? > # requested upstream add license https://github.com/beanbaginc/kgb/issues/10 > Source1: LICENSE Where is this file coming from in the meantime? > BuildRequires: python%{python3_pkgversion}-devel > BuildRequires: python%{python3_pkgversion}-setuptools > # Test dependencies: > BuildRequires: python3dist(pytest) > BuildRequires: python3dist(six) You choose to make the spec file more complex by using the %{python3_pkgversion} macro instead of literally using 3. But then you use python3dist(pytest) directly, discarding any potential benefit of using %{python3_pkgversion} in the first place. What is your motivation for using %{python3_pkgversion}? What EPEL versions and Python versions in that EPEL do you target here? Is it needed, or is it just copy-pasted from some other spec file? > %{?python_provide:%python_provide python3-%{srcname}} This is deprecated and SHOULD NOT be used. See the note at the end of https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/#_the_py_provides_macro > %if 0%{?fedora} || 0%{?rhel} > 8 > ... > %if 0%{?el8} The spec seems to mix two kinds of conditionals that mean the same in this context. I highly recommend choosing one and sticking to it. The old/new Python packaging style %id/%else is already hard to follow as it is. > %if 0%{?fedora} || 0%{?rhel} > 8 > %generate_buildrequires > %pyproject_buildrequires > %endif What is the benefit of using this, considering the package already needs to list all the deps for EPEL 8? > # move local module folder out of the way so pytest doesn't use the wrong one > %pytest --pyargs %{srcname} The comment does not seem to correspond to the actual usage. Is it outdated? -- 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=2114603 _______________________________________________ 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
