Bug 2055323 - Review Request: rust-virtio-queue - Virtio queue implementation
Summary: Review Request: rust-virtio-queue - Virtio queue implementation
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-02-16 17:16 UTC by Sergio Lopez
Modified: 2022-03-10 15:12 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-03-10 15:12:37 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Comment 1 Fabio Valentini 2022-02-16 17:26:10 UTC
Two quick notes from Rust packaging POV:

1. The package and crate does not contain license files.
You should poke upstream project to include them in published crates.
Until that is resolved, you will probably need to include the two license files as additional SOURCEs and add them manually.

2. %cargo_test "--features" test-utils

This looks weird, not sure if that even works as expected.
The %cargo_test macro even has an "-f" flag for this exact use case:

%cargo_test -f test-utils

Comment 2 Sergio Lopez 2022-02-16 18:16:08 UTC
(In reply to Fabio Valentini from comment #1)
> Two quick notes from Rust packaging POV:
> 
> 1. The package and crate does not contain license files.
> You should poke upstream project to include them in published crates.
> Until that is resolved, you will probably need to include the two license
> files as additional SOURCEs and add them manually.

That's the result of this crate being in a shared repository. I'll let upstream know. I think fixing it should be as simple as adding some "license_file" entries in this crate's Cargo.toml.

I've added the files as addition SOURCEs, pointing to URLs where the license files reside in the shared repository.

> 2. %cargo_test "--features" test-utils
> 
> This looks weird, not sure if that even works as expected.
> The %cargo_test macro even has an "-f" flag for this exact use case:
> 
> %cargo_test -f test-utils

It works, but I agree it's weird. I've fixed it too.

https://download.copr.fedorainfracloud.org/results/slp/rust-virtio-queue/fedora-36-x86_64/03517131-rust-virtio-queue/rust-virtio-queue.spec
https://download.copr.fedorainfracloud.org/results/slp/rust-virtio-queue/fedora-36-x86_64/03517131-rust-virtio-queue/rust-virtio-queue-0.1.0-1.fc36.src.rpm

Thanks for the review!

Comment 3 Sergio Lopez 2022-02-22 13:44:45 UTC
Fabio, could you please confirm if I have addressed properly your concerns?

Thanks!
Sergio.

Comment 4 Fabio Valentini 2022-02-22 13:59:38 UTC
Yes. Sorry, I did not mean to start a formal review, because I'm quite short on time recently.
But to not keep you waiting any further, I've finished the review regardless.
The package looks good now. Just make sure to alert the upstream project about the missing license files.
You might also want to create bugs for the missing architecture support, and make them block the appropriate tracker/blocker bugs.

===

Package was generated with rust2rpm, simplifying the review.

- package builds and installs without errors on rawhide
- test suite is run and all unit tests pass
- latest version of the crate is packaged
- license matches upstream specification and is acceptable for Fedora
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- add @rust-sig with "commit" access as package co-maintainer

- set bugzilla assignee overrides to @rust-sig (optional)

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- track package in koschei for all built branches

Comment 5 Gwyn Ciesla 2022-02-22 22:41:57 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-virtio-queue

Comment 6 Sergio Lopez 2022-03-10 15:12:37 UTC
Package has hit the compose. Thanks Fabio and Gwyn!


Note You need to log in before you can comment on or make changes to this bug.