https://bugzilla.redhat.com/show_bug.cgi?id=2006997
Bug ID: 2006997 Summary: Review Request: rust-wasmtime-types - WebAssembly type definitions for Cranelift Product: Fedora Version: rawhide Status: NEW Component: Package Review Assignee: nobody@fedoraproject.org Reporter: o.lemasle@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://olem.fedorapeople.org/rust-wasmtime-types.spec SRPM URL: https://olem.fedorapeople.org/rust-wasmtime-types-0.30.0-1.fc35.src.rpm
Description: WebAssembly type definitions for Cranelift.
Fedora Account System Username: olem
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
Olivier Lemasle o.lemasle@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value
--- Comment #1 from Olivier Lemasle o.lemasle@gmail.com --- Scratch build (in f36-build-side-46075): https://koji.fedoraproject.org/koji/taskinfo?taskID=76125667
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Status|NEW |ASSIGNED CC| |decathorpe@gmail.com
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- I'll take this review.
Package looks good (generated with rust2rpm without modifications), but there's one blocking issue (from my point of view):
The crate uses a nonstandard license (Apache-2.0 WITH LLVM-Excpetion), for which we'd need to have the actual license text included in published crates, and then included in the `-devel` subpackage of this crate with "%license".
(Even if the project used the standard Apache-2.0 license, including the license text with redistributed sources is *mandatory* according to the license terms. So upstream technically isn't even in compliance with the terms of their own chosen license, since they publish and redistribute it via crates.io *without* the license text ...)
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
--- Comment #3 from Olivier Lemasle o.lemasle@gmail.com --- You're right, thanks. I had indeed previously added the LICENSE file in some other wasmtime crates; I forgot to add it here.
I've opened a PR upstream: https://github.com/bytecodealliance/wasmtime/pull/3382
Should I include the LICENSE file directly in the srpm in the meantime?
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
--- Comment #4 from Fabio Valentini decathorpe@gmail.com ---
Should I include the LICENSE file directly in the srpm in the meantime?
Yes, please. Just include the same file that you added with the PR, once it's merged. The file can then be removed in favor of the "real one" as soon as a published version has it.
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
--- Comment #5 from Olivier Lemasle o.lemasle@gmail.com --- I've updated the package to include the license file:
Spec URL: https://olem.fedorapeople.org/reviews/rust-wasmtime-types.spec SRPM URL: https://olem.fedorapeople.org/reviews/rust-wasmtime-types-0.30.0-2.fc35.src....
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags| |fedora-review+
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- Looks good to me now. You didn't have to include a separate changelog entry for this small change :)
=============
Final review:
Simplified review, because this is a standard Rust package that was generated with rust2rpm.
- package builds successfully on rawhide (see koji scratch build) - test suite is run and all unit tests pass (there are no tests in this crate) - latest version of the crate is packaged - license matches upstream specification and is acceptable for Fedora (ASL 2.0 with exceptions) - license file has been submitted upstream and is included with %license in %files - package complies with Rust Packaging Guidelines
Package APPROVED.
Please don't forget to also request f35 and f34 branches for your package.
(Pro tip: You can do that before the repository exists by using the "--repo" flag for "fedpkg request-branch, since these requests are processed chronologically. So this works fine so long as you request the repository first, and any branches second. This reduces the waiting time from two to one round-trip-time with the fedora-scm-requests ticket queue.)
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
--- Comment #7 from Olivier Lemasle o.lemasle@gmail.com --- Thanks Fabio for your detailed review and the pro tip!
https://pagure.io/releng/fedora-scm-requests/issue/37037 https://pagure.io/releng/fedora-scm-requests/issue/37038 https://pagure.io/releng/fedora-scm-requests/issue/37039
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
--- Comment #8 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-wasmtime-types
https://bugzilla.redhat.com/show_bug.cgi?id=2006997
Olivier Lemasle o.lemasle@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |RAWHIDE Status|POST |CLOSED Last Closed| |2021-09-22 22:39:09
--- Comment #9 from Olivier Lemasle o.lemasle@gmail.com --- Package now in rawhide: https://bodhi.fedoraproject.org/updates/FEDORA-2021-ef80258c07
package-review@lists.fedoraproject.org