https://bugzilla.redhat.com/show_bug.cgi?id=2510861
--- Comment #11 from Kenny Glowner <[email protected]> --- (In reply to Fabio Valentini from comment #10) > Just a few drive-by comments (not committing to a full review): > > - Don't use "if fedora else <...>" conditionals. > This is a package under review *for Fedora*, the "else" condition would be > dead code. > > - Missing documentation for which licenses apply to what (this project / > vendored dependencies / etc.). > This would usually be handled by the %cargo_license_summary and > %cargo_license macros. > > "GPL-2.0-or-later AND MIT AND Apache-2.0" is not the correct / complete > License tag, > it needs to cover all items from the summary printed during the build: > > ### BEGIN LICENSE SUMMARY ### > # Apache-2.0 OR MIT > # Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT > # BSD-2-Clause OR Apache-2.0 OR MIT > # GPL-2.0-or-later > # MIT > # MIT OR Apache-2.0 > # MIT OR Apache-2.0 OR LGPL-2.1-or-later > # MIT OR LGPL-3.0-or-later > # Unlicense OR MIT > ### END LICENSE SUMMARY ### > > - Why use "Epoch: 1"? This is a new package, it should not need an Epoch yet. > > - Why use vendored dependencies? > Fedora packages "SHOULD NOT" use vendored dependencies, unless adding > required dependencies to Fedora would require an unreasonable amount of work. > If you argue for "unreasonable amount of work", then that needs to be > documented. > > - Use rpmautospec (%autorelease / %autochangelog). > This is a "SHOULD" guideline too. > > In general, I would recommend that you look at the "non-crate Rust package" > template, and the Rust packaging Guidelines in general: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/ > #_non_crate_rust_project Hi Fabio, Thanks for the drive-by review and pointers, much appreciated! I've gone ahead and made the requested adjustments to align with the Fedora Rust packaging guidelines: - Replaced the static release and changelog with rpmautospec (%autorelease and %autochangelog). - Updated the License tag to reflect the comprehensive, combined list of licenses output by the %cargo_license_summary macro, along with an explicit breakdown of the vendor licenses in the comments. - Dropped the Epoch: 1 tag since this is a new package and it's not strictly necessary. - Added explicit documentation detailing why vendored dependencies are used (packaging the entire dependency tree for Fedora initially would require an unreasonable amount of work). Regarding the if fedora else <...> conditionals: I have retained the %if 0%{?fedora} / %else structure because this exact same spec file is used to build tuned-rs for EPEL/RHEL targets in our upstream Copr repository, which lacks native cargo-rpm-macros support. The else block is entirely dead code when building within Fedora, so it has zero impact on the Fedora build process. Given that the Fedora ecosystem and this review process are so closely tied to RHEL, I'm sure the necessity of maintaining EPEL compatibility in a shared spec file makes perfect sense. The updated spec and SRPM from the new Copr build are linked below. Let me know if anything else catches your eye! Spec URL: https://download.copr.fedorainfracloud.org/results/sisyphuscode/tuned-rs/fedora-rawhide-x86_64/10876929-tuned-rs/tuned-rs.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/sisyphuscode/tuned-rs/fedora-rawhide-x86_64/10876929-tuned-rs/tuned-rs-0.2.10-1.fc46.src.rpm [fedora-review-service-build] -- 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=2510861 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202510861%23c11 -- _______________________________________________ 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
