https://bugzilla.redhat.com/show_bug.cgi?id=2274028 Fabio Valentini <decathorpe@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(decathorpe@gmail. | |com) | --- Comment #8 from Fabio Valentini <decathorpe@xxxxxxxxx> --- Thank you! Looks much better now, just three minor things left from my side: 1. I wonder why you added "as i32" in the first line of the patch compared to what I suggested? Looking at the code for the types of the variables involved, it's unnecessary on both 64-bit and 32-bit architectures. 2. The output of "%cargo_license_summary" is still missing from the spec file (i.e. the itemized list of licenses, not their AND-combined result). 3. Tests are skipped with "test does not panic" as only explanation. What does this mean? The only reasons where tests won't panic as expected should be when they either mistakenly use debug_assertions instead of normal assertions, or rely on "panic on integer overflow" behaviour that is specific to "debug" build mode. If either of these two issues is the cause for some tests not failing as expected, please document it. If tests fail for other (valid) reasons, it would be good to file an upstream ticket about it. -- 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=2274028 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202274028%23c8 -- _______________________________________________ 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
