https://bugzilla.redhat.com/show_bug.cgi?id=2401972
--- Comment #9 from Benson Muite <[email protected]> --- Thanks for the feedback. (In reply to Ben Beasley from comment #8) > Overall, this looks really good! There are just a few things that need a > second > look. > > Issues: > ======= > - Dist tag is present. > > OK: fedora-review is confused by rpmautospec > > - It would be better to remove the bundled HPGCC in the benchmarks in %prep > to > prove that it is not used in the build. > > # Remove bundled HPGCC, only for benchmarks, to show it is unused > rm -rv test/benchmarks/mantevo/hpccg/ > Done > - Contents of test/benchmarks/mantevo/hpccg/ are a bundled copy of HPGCC > under > LGPL-2.1-or-later, which is of course acceptable in Fedora. Since these > sources do not contribute to the binary RPMs, this term is correctly > omitted > from License. > > - The file COPYING contains three licenses. The first two are BSD-3-Clause > and > MIT, and are correctly represented in License. The third appears to be > https://spdx.org/licenses/dtoa.html. Therefore, I belive the License should > be: > > # Breakdown of licenses is documented in COPYING. > License: BSD-3-Clause AND MIT AND dtoa Done > > (You don’t have to add the comment if you don’t want to.) > > - This: > > %{_libdir}/libqthread.so.1* > > can be more precisely written as: > > %{_libdir}/libqthread.so.1{,.*} Done > > which would not match e.g. libqthread.so.14. See: > > > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_listing_shared_library_files > > This is a small quibble that’s unlikely to make a difference in practice. > > - For ExcludeArch: s390x, you need to document the reason in (for now) a > comment. You sort of have this with > > # Unsupported architectures > > but that doesn’t really add any useful information. Maybe something like > this? > > # This library relies on some low-level platform-specific code, so it can > # only be expected to work on explicitly-supported architectures, and it > only > # compiles on architectures that upstream has attempted to support. See: > # > # > https://github.com/sandialabs/qthreads/blob/1.22/README.md#compatibility > # > https://github.com/sandialabs/qthreads/blob/1.22/include/qthread/common. > h#L44-L58. > # > # https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval > ExcludeArch: %{ix86} s390x Done > > When you import the package, you must file a tracking bug with the details > about the architecture incompatibility, make it block F-ExcludeArch-s390x, > and link it from the spec-file comment; see > > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_architecture_build_failures. > If you like, you can then omit the description from the spec file and just > link the bug. > > A tracking bug isn’t required for i686 due to > https://fedoraproject.org/wiki/Changes/EncourageI686LeafRemoval. > > - The source includes API documentation in the form of man pages. You might > consider adding these to the -devel subpackage: > > %install > […] > install -t '%{buildroot}%{_mandir}/man3' -p -m 0644 -D man/man3/*.3 > > […] > > %files devel > %{_mandir}/man3/q*.3* > […] Done spec: https://fed500.fedorapeople.org/qthreads.spec srpm: https://fed500.fedorapeople.org/qthreads-1.22%5e20251007gitcedb1fc-1.fc42.src.rpm spec: srpm: -- 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=2401972 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202401972%23c9 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected] Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
