https://bugzilla.redhat.com/show_bug.cgi?id=2159976 Miro Hrončok <mhroncok@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok@xxxxxxxxxx --- Comment #2 from Miro Hrončok <mhroncok@xxxxxxxxxx> --- From https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2159976-python-scikit-build/fedora-rawhide-x86_64/05216165-python-scikit-build/fedora-review/review.txt > python3-scikit-build.x86_64: E: no-binary This indicates this should be a noarch package. The need for `%global debug_package %{nil}` indicates the same. > python-scikit-build.spec:1: W: macro-in-comment %files That is in the comment about debug_package and will go away. > python-scikit-build.src: E: description-line-too-long cross compilation, and locating dependencies and determining their build requirements. > python3-scikit-build.x86_64: E: description-line-too-long cross compilation, and locating dependencies and determining their build requirements. Please, wrap the lines. ========= Spec comments: > BuildRequires: gcc-c++ > BuildRequires: gcc-gfortran I suggest also adding gcc. It is transitively pulled but used directly. I also suggest separating the tests deps visually from the build deps e.g.: BuildRequires: python3-devel # For tests: BuildRequires: cmake BuildRequires: gcc BuildRequires: gcc-c++ BuildRequires: gcc-gfortran BuildRequires: git-core BuildRequires: ninja-build > Requires: cmake Is ninja also required on runtime or not? > Patch: 0001-Remove-test-deps-missing-in-Fedora.patch Consider not numbering the patch file, please. When/if more patches are added and some are removed, it gets messy. We got rid of the PatchX: numbering recently, let's not dig ourselves back with patch filenames? > %pytest -k "not pep518 and not \ > test_hello_sdist and not \ > test_hello_sdist_with_base and not \ > test_sdist_with_symlinks and not \ > test_manifest_in_sdist" -m "not deprecated" Consider putting -m "not deprecated" on a separate line after backslash, IMHO it improves readability. -- 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=2159976 _______________________________________________ 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
