Bug 1990158

Summary: Review Request: rust-exitfailure - Basic newtype wrappers for use with ? in main
Product: [Fedora] Fedora Reporter: Davide Cavalca <davide>
Component: Package ReviewAssignee: Fabio Valentini <decathorpe>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: decathorpe, package-review
Target Milestone: ---Flags: decathorpe: fedora-review+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: rust-exitfailure-0.5.1-1.fc36 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2021-09-03 14:59:45 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1990648    

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