https://bugzilla.redhat.com/show_bug.cgi?id=2141068
Bug ID: 2141068 Summary: Review Request: rust-test-generator - Rust Test generator: enumerating entries according to file-system pattern and generating a test function for each entry Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: klember@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://kalev.fedorapeople.org/rust-test-generator.spec SRPM URL: https://kalev.fedorapeople.org/rust-test-generator-0.3.0-1.fc38.src.rpm Description: Rust Test generator: enumerating entries according to file-system pattern and generating a test function for each entry.
Fedora Account System Username: kalev
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93950912
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com Flags| |fedora-review? Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Status|NEW |ASSIGNED
--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- Taking this review.
Initial comments:
- Please don't ignore big FIXMEs in specs generated by rust2rpm.
In this case, Apache-2.0 license file is missing. Please report this upstream.
- The summary is way too long, please trim it to something sensible like
"Generate test functions for all files matching a pattern" or something like that (or you can come up with something better - the current description is kind of confusing and I don't really understand what it's trying to say).
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
--- Comment #2 from Kalev Lember klember@redhat.com --- (In reply to Fabio Valentini from comment #1)
Taking this review.
Initial comments:
- Please don't ignore big FIXMEs in specs generated by rust2rpm.
In this case, Apache-2.0 license file is missing. Please report this upstream.
Done: https://github.com/frehberg/test-generator/issues/15
- The summary is way too long, please trim it to something sensible like
"Generate test functions for all files matching a pattern" or something like that (or you can come up with something better - the current description is kind of confusing and I don't really understand what it's trying to say).
Thanks, I've updated the spec file to use your suggestion. I don't think it really matters much as the Fedora packaging is just for use in koji and I imagine everybody else is just getting the crates from crates.io directly.
I wonder if maybe we should update rust2rpm to generate a shorter summary and leave the upstream text for description? Like,
Summary: rust "%{crate}" crate %global _description %{expand: Rust Test generator: enumerating entries according to file-system pattern and generating a test function for each entry.}
%description %{_description}
Something like this would allow for a bit more automation :)
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
--- Comment #3 from Kalev Lember klember@redhat.com --- * Tue Nov 15 2022 Kalev Lember klember@redhat.com 0.3.0-3 - Shorten the summary (#2141068)
* Tue Nov 15 2022 Kalev Lember klember@redhat.com 0.3.0-2 - Re-generate the spec file with rust2rpm 23
Spec URL: https://kalev.fedorapeople.org/rust-test-generator.spec SRPM URL: https://kalev.fedorapeople.org/rust-test-generator-0.3.0-3.fc38.src.rpm
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=94207207
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
--- Comment #4 from Fabio Valentini decathorpe@gmail.com ---
Something like this would allow for a bit more automation :)
Trimming the Summary automatically already happens. The Cargo.toml package.description contents are trimmed to the first line, or the first "." character, if I remember correctly. But that heuristic obviously doesn't work if the upstream description starts with a sentence that spans a few pages ...
It looks like upstream project *does* have license files in its git repo. Please include these files manually for now. Both Apache-2.0 and MIT require redistributed sources to contain a copy of the license text.
Also note that this project seems to be rather ... cruft-y (or just very old and not updated). Its dependencies are all ancient crate versions, and compiling the code prints lots of deprecation warnings ...
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
--- Comment #5 from Kalev Lember klember@redhat.com --- (In reply to Fabio Valentini from comment #4)
It looks like upstream project *does* have license files in its git repo. Please include these files manually for now. Both Apache-2.0 and MIT require redistributed sources to contain a copy of the license text.
Thanks, fixed. I only added LICENSE-APACHE though because the MIT license that's in the root of the upstream repo appears to be for another crate that happens to be in the same git repo, in another subdir, and not applicable to this one.
Also note that this project seems to be rather ... cruft-y (or just very old and not updated). Its dependencies are all ancient crate versions, and compiling the code prints lots of deprecation warnings ...
Yes :( Looks like upstream stopped maintaining it a few years ago.
* Fri Nov 18 2022 Kalev Lember klember@redhat.com 0.3.0-4 - Add missing LICENSE-APACHE file from upstream (#2141068)
Spec URL: https://kalev.fedorapeople.org/rust-test-generator.spec SRPM URL: https://kalev.fedorapeople.org/rust-test-generator-0.3.0-4.fc38.src.rpm
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=94303698
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- Oh, right, this crate is Apache-2.0 ONLY, while other crates in the same repo are MIT/Apache-2.0, resulting in license files for both being present - which confused me. Package Looks good to me now.
===
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 (manually included from upstream repo) - 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
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
--- Comment #7 from Tomas Hrcka thrcka@redhat.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-test-generator
https://bugzilla.redhat.com/show_bug.cgi?id=2141068
Kalev Lember klember@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |RAWHIDE Status|POST |CLOSED Fixed In Version| |rust-test-generator-0.3.0-4 | |.fc37 | |rust-test-generator-0.3.0-4 | |.fc38 Last Closed| |2022-11-21 12:49:30
--- Comment #8 from Kalev Lember klember@redhat.com --- Thanks, Fabio! Package imported and built.
package-review@lists.fedoraproject.org