https://bugzilla.redhat.com/show_bug.cgi?id=2228632 Fabio Valentini <decathorpe@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value CC| |decathorpe@xxxxxxxxx --- Comment #3 from Fabio Valentini <decathorpe@xxxxxxxxx> --- I've got two comments, one from a Rust specific point of view, one from a general packaging view: 1. The "criterion" dev-dependency is (almost exclusively) only used for building / running benchmarks. We don't compile / run benchmarks during package builds (this is strongly discouraged, just like running linters). So this dependency is actually unused, and can be removed/patched out from Cargo.toml (i.e. using "rust2rpm -sp"). Doing that should allow you to build the package with the test suite enabled (i.e. flip the "check" bcond at the top again). 2. It's always good to document some decisions, like a) why are tests disabled, or b) what does this patch do, etc. In this case, it would be good to document either "tests are disabled because of a missing dependency" or, after fixing 1., "the criterion dev-dependency is only used for benchmarks / unused and can be dropped". -- 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=2228632 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202228632%23c3 _______________________________________________ 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
