Bug 1990158 - Review Request: rust-exitfailure - Basic newtype wrappers for use with ? in main
Summary: Review Request: rust-exitfailure - Basic newtype wrappers for use with ? in main
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1990648
TreeView+ depends on / blocked
 
Reported: 2021-08-04 21:27 UTC by Davide Cavalca
Modified: 2021-09-03 14:59 UTC (History)
2 users (show)

Fixed In Version: rust-exitfailure-0.5.1-1.fc36
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-09-03 14:59:45 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description Davide Cavalca 2021-08-04 21:27:43 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rust-exitfailure/rust-exitfailure.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rust-exitfailure/rust-exitfailure-0.5.1-1.fc35.src.rpm

Description:
Basic newtype wrappers for use with ? in main.

Fedora Account System Username: dcavalca

Comment 1 Davide Cavalca 2021-08-04 21:27:45 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=73293740

Comment 2 Fabio Valentini 2021-08-26 10:52:56 UTC
Taking this review.

Some small blockers:

- open a PR with upstream about updating your patched dependencies and/or link the PR in the spec file
- explain why you need to "sed" the tests/test.rs file (also, this test should probably use something like `env!("CARGO_TARGET_DIR")` (don't quote me on that) instead of hard-coding "target/debug" or "target/release" ...)
- why is the "test_backtrace" test disabled?

Minor nitpicks:

- leave the generated "# Initial patched metadata" comment in the .spec file
- drop "examples" and maybe even "CONTRIBUTING.md" from "%doc", both are pretty useless for RPM packages

Comment 3 Davide Cavalca 2021-08-26 16:38:10 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rust-exitfailure/rust-exitfailure.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rust-exitfailure/rust-exitfailure-0.5.1-1.fc36.src.rpm

Changelog:
- add upstream PRs to comment for patched dependencies
- drop useless docs
- extend comment for tests/test.rs sed; I tried fixing this properly with CARGO_TARGET_DIR but that doesn't seem to be propagated down to the tests so it won't work
- mention upstream issue when disabling test_backtrace

Comment 4 Fabio Valentini 2021-08-26 16:56:12 UTC
Thanks, looks good 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 (with one reported exception)
- latest version of the crate is packaged
- patches are justified and submitted upstream
- 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.

Comment 5 Davide Cavalca 2021-08-26 17:03:12 UTC
Thanks!

$ fedpkg request-repo rust-exitfailure 1990158
https://pagure.io/releng/fedora-scm-requests/issue/36562

Comment 6 Gwyn Ciesla 2021-08-26 18:31:10 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-exitfailure


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