https://bugzilla.redhat.com/show_bug.cgi?id=1942392 --- Comment #3 from Sergio Lopez <slopezpa@xxxxxxxxxx> --- (In reply to Connor Kuehl from comment #2) > Hi Sergio! Hi, Connor. Thanks for the review! > Overall it looks good. Just a few requested changes: > > 1. The vmm-sys-util-fix-metadata.diff should be sent upstream, otherwise > their crates.io package is needlessly including that CI stuff Sure, I intend to do that for vmm-sys-util and, pretty much, every other rust-vmm crate. > 1.1. This diff doesn't do what it looks like it should do. Since upstream > doesn't exclude this in their manifest, patching it out here is too late, as > it's already included in the *.crate source tarball. I suspect you'll want > to update this patch to actually remove the rust-vmm-ci/ directory and the > /coverage_config_* files. This is indicated in the Rust Packaging Guidelines [1] and other packages, such as rust-memmap, include similar patches. I've tested it and doing this prevents those files to be included in the -devel package. [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_excluding_unnecessary_files > 2. I think the other patch, vmm-sys-util-omit-ioctl-tests.diff, is fine, but > if you wanted to, you can automatically patch out the tests by changing > '%bcond_without check' to '%bcond_with check' in the .spec file which would > mean that this patch could also be removed. This isn't a required change for > this review though. But that would disable all tests, wouldn't it? > 3. The %license field is missing > > 4. The changelog is slightly different than one of the prescribed formats > (https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs). > I've been wondering if rust2rpm should stop putting the HH:MM:SS timestamp > in the changelog entry. It's on my list to ask the other Rust packagers + > upstream rust2rpm. Ouch. I'll fix both right away. -- 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 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure