[Bug 2055323] New: Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

Bug ID: 2055323
           Summary: Review Request: rust-virtio-queue - Virtio queue
                    implementation
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL:
https://download.copr.fedorainfracloud.org/results/slp/rust-virtio-queue/fed...
SRPM URL:
https://download.copr.fedorainfracloud.org/results/slp/rust-virtio-queue/fed...
Description:
Virtio queue implementation

Fedora Account System Username: slp

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Fabio Valentini  ---
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

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

--- Comment #2 from Sergio Lopez  ---
(In reply to Fabio Valentini from comment #1)
> Two quick notes from Rust packaging POV:
> 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.
> %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/fed...
https://download.copr.fedorainfracloud.org/results/slp/rust-virtio-queue/fed...

Thanks for the review!

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

Sergio Lopez  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(decathorpe@gmail.
                   |                            |com)

--- Comment #3 from Sergio Lopez  ---
Fabio, could you please confirm if I have addressed properly your concerns?

Thanks!
Sergio.

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(decathorpe@gmail. |fedora-review+
                   |com)                        |
             Status|NEW                         |POST
           Assignee|[e-mail]    |[e-mail]

--- Comment #4 from Fabio Valentini  ---
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

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

--- Comment #5 from Gwyn Ciesla  ---
(fedscm-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/rust-virtio-queue

[Bug 2055323] Review Request: rust-virtio-queue - Virtio queue implementation

https://bugzilla.redhat.com/show_bug.cgi?id=2055323

Sergio Lopez  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |RAWHIDE
             Status|POST                        |CLOSED
        Last Closed|                            |2022-03-10 15:12:37

--- Comment #6 from Sergio Lopez  ---
Package has hit the compose. Thanks Fabio and Gwyn!