Bug 1921853 - Review Request: rust-derive-new - Derive simple constructor functions for structs and enums
Summary: Review Request: rust-derive-new - Derive simple constructor functions for str...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: rust-derive-new (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-01-28 17:39 UTC by Sohan Kunkerkar
Modified: 2021-08-03 16:24 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-02-22 11:56:31 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description Sohan Kunkerkar 2021-01-28 17:39:11 UTC
Spec URL: https://sohank2602.fedorapeople.org/rust-derive-new/rust-derive-new.spec
SRPM URL: https://sohank2602.fedorapeople.org/rust-derive-new-0.5.8-1.fc33.src.rpm
Description: Derive simple constructor functions for structs and enums
Fedora Account System Username: sohank2602

Comment 1 Fabio Valentini 2021-01-29 11:15:08 UTC
First comment:

The Summary tag (taken from upstream metadata) is really long, you should probably change it to
"Derive simple constructor functions for structs and enums"
or similar.

And please also use this Summary as the Summary of the Review Request template (in the bug title).

I'll make a full review this afternoon.

Comment 2 Robert-André Mauchin 🐧 2021-01-29 22:50:04 UTC
Please ask upstream to include the license file in their crate.

Comment 3 Sohan Kunkerkar 2021-02-01 06:32:45 UTC
(In reply to Robert-André Mauchin 🐧 from comment #2)
> Please ask upstream to include the license file in their crate.

It seems like the upstream project is not maintained for a while. The last commit was added in 2018. I tried to reach the maintainer of the project, but haven't received any reply yet.

Comment 4 Sohan Kunkerkar 2021-02-09 02:36:06 UTC
(In reply to Robert-André Mauchin 🐧 from comment #2)
> Please ask upstream to include the license file in their crate.

Alright, the license file is updated in the new crate.

Also, some changes in the original content:
Spec URL: https://sohank2602.fedorapeople.org/rust-derive-new/rust-derive-new.spec
SRPM URL: https://sohank2602.fedorapeople.org/rust-derive-new-0.5.9-1.fc33.src.rpm

Comment 5 Fabio Valentini 2021-02-09 12:46:06 UTC
A LICENSE file is now in the upstream crate, but it's still not added in the package.

Add "%license LICENSE" to the %files list for the -devel subpackage.


Also please remove the markdown markup (the two `) from the Summary tag.

Comment 6 Sohan Kunkerkar 2021-02-09 13:59:21 UTC
(In reply to Fabio Valentini from comment #5)
> A LICENSE file is now in the upstream crate, but it's still not added in the
> package.
> 
> Add "%license LICENSE" to the %files list for the -devel subpackage.
> 
> 
> Also please remove the markdown markup (the two `) from the Summary tag.

Thanks for the review! 

Fixed. Could you take another look?

Comment 7 Fabio Valentini 2021-02-09 16:55:20 UTC
Uhm ... What did you do to Summary and description?
I said remove the two `s, not to remove everything between them :)
The text itself was fine, just the markup makes no sense in RPM tags ...

If you *want* to change the Summary text, do something like this to make it make sense:
Summary:        Derive simple constructor functions for structs and enums

The %description can stay unmodified from the rust2rpm generated .spec.

Other than that: package generated with rust2rpm, simplifying the review:

- latest version from crates.io is packaged
- License matches upstream specification and shipped as %license
- follows Packaging and Rust Packaging Guidelines
- builds and installs successfully on rawhide

Please fix the Summary and %description tags before importing and building the package.

Comment 8 Sohan Kunkerkar 2021-02-09 17:35:09 UTC
> Please fix the Summary and %description tags before importing and building the package.

My bad, it's fixed in the subsequent push.

Comment 9 Mohan Boddu 2021-02-12 14:38:05 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-derive-new

Comment 10 Sohan Kunkerkar 2021-02-22 11:56:31 UTC
This package is successfully built for F-34 https://koji.fedoraproject.org/koji/buildinfo?buildID=1711325

Thanks Fabio Valentini for the help!

Comment 11 Mattia Verga 2021-08-03 16:24:24 UTC
*** Bug 1873737 has been marked as a duplicate of this bug. ***


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