[Bug 1942392] Review Request: rust-vmm-sys-util - Helpers and utilities used by multiple rust-vmm components and VMMs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux