https://bugzilla.redhat.com/show_bug.cgi?id=2095005 Maxwell G <gotmax@e.email> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags| |fedora-review? Assignee|nobody@xxxxxxxxxxxxxxxxx |gotmax@e.email CC| |gotmax@e.email Doc Type|--- |If docs needed, set a value --- Comment #1 from Maxwell G <gotmax@e.email> --- This looks prety good. I have a couple minor, nitpicky comments: > BuildRequires: python3dist(pytest-runner) > BuildRequires: [...] 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. There's a tox env that pulls in testing dependencies, but it's not particularly useful due to pinning and extra stuff like coverage or twine. 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. [0]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_canonical_project_name > %global common_description %{expand: \ > A pure python RFC3339 validator.} If you're trying to escape the newline here, this doesn't work as expected. Compare the value of rpm -qp --qf="%{description}\n https://music.fedorapeople.org/python-rfc3339-validator-0.1.4-1.fc36.src.rpm"; to any other RPM and you'll notice an extra leading newline. You should use the standard `%description -n python3-rfc3339-validator %{common_description}`. Another option is to put `%wordwrap -v common_description` on a new line below `%description`. The is a lesser known and not very well documented macro that[1]: > -- Reformat a text intended to be used used in a package description, removing > -- rpm macro generation artefacts. > -- – remove leading and ending empty lines > -- – trim intermediary empty lines to a single line > -- – fold on spaces Am I being overly pedantic? Probably. Anyways... [1]: https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/common.lua#_163 It looks like the unit tests are being installed to `%{python3_sitelib}`. Is that something we want? -- 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
