https://bugzilla.redhat.com/show_bug.cgi?id=2480195
--- Comment #1 from [email protected] --- Some broad strokes. # SPDX-License-Identifier: Apache-2.0 # Spec layout mirrors src/debian/control and src/debian/rules. # Assisted-by: Generic LLM chatbot Fedora's default license is MIT, Apache is fine. Remove the next two lines, no one will have the context of the debian package. Assisted-by: and any other signoff should be in the git commit log and can be done later. Release: 1%{?dist} consider using the Release: %autorelease and later %changelog %autochangelog Summary: AMD Xilinx FPGA and ACAP runtime (XRT) I usually copy the 'about' section of the github page consider using Summary: Run Time for AIE and FPGA based platforms License: Apache-2.0 AND MIT AND MIT-Khronos-old # License breakdown: # - Core XRT runtime: Apache-2.0 # - AIE binary utilities: MIT # - Core XRT OpenCL library: Apache-2.0 and MIT-Khronos-old Review the fedorareview output file licensecheck. A lot of licenses were not captured in the license: tag The breakdown of the licenses is too brief. There are many license.txt or similar files not captured in the later %license in file If code is not used, remove it in the %prep stage to make the license review easier. Patch102: license.patch remove this patch. The full licenses are fine and any license changes should be done in the upstream not in distro patches. # Debian patches # Fedora patches These are all fedora patches now, these comments are not needed Patch0: 6.18.patch Patch1: 6.19.patch Patches should have a comment on what the patch does and if possible reference the upstream issue or pr this is a general problem. I find using git format patches more helpful as any updated to the package will require rebasing. consider reworking these patches to be git friendly. # Man pages not installed by CMake Source10: aiebu-asm.1 Source11: aiebu-dump.1 How are these man pages generated ? I see from aiebu-asm SEE ALSO The full documentation for aiebu-asm is maintained as a Texinfo manual. If the info and aiebu-asm programs are properly in‐ stalled at your site, the command info aiebu-asm they reference info, which afaik isn't in this package so these man pages will confuse users. ExclusiveArch: aarch64 x86_64 Does this package and all its features work on aarch64 ? hip i know is only x86_64. BuildRequires: cmake >= 3.16 only 1 version of cmake, this is not needed. The reset of the buildrequires looks autogenerated. review what is actually needed and put then in a better order. The packages are a matter of taste. Mine is to limit the packages I think the xrt-utils-* could be collapsed to the main package. Similar for xrt-npu package. For consistency with ROCm the programming interfaces opencl and hip should be xrt-opencl, xrt-opencl-devel and xrt-hip and xrt-hip-devel %cmake \ -DCMAKE_BUILD_TYPE=Release \ Release should change to RelWithDebInfo %check does this really work without hw ? %files ... %{_libdir}/libxrt++.so.* This globbing it too aggressive. At least the major so version needs to be captured. consider something like https://src.fedoraproject.org/rpms/rocclr/blob/rawhide/f/rocclr.spec#_440 -- 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=2480195 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202480195%23c1 -- _______________________________________________ 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://forge.fedoraproject.org/infra/tickets/issues/new
