https://bugzilla.redhat.com/show_bug.cgi?id=2440262



--- Comment #28 from Tyler Fanelli <[email protected]> ---
(In reply to Ben Beasley from comment #21)
> Thanks for the update! This is getting really close.
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> 
> ===== Issues =====
> 
> - The License tag for the (sub)package with a compiled binary – in this case,
>   for the base package – needs to include the licenses of Rust crate
> libraries
>   that are statically linked into the executable.
> 
>   See
>  
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/
> #_license_tags,
>   and also the template
>  
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/
> #_non_crate_rust_project.
> 
>   When you do a build of the package, observe the output from
>   %{cargo_license_summary}. For this package, that looks like:
> 
>     ### BEGIN LICENSE SUMMARY ###
>     # Apache-2.0
>     # Apache-2.0 OR MIT
>     # MIT
>     # MIT OR Apache-2.0
>     # Unlicense OR MIT
>     ###  END LICENSE SUMMARY  ###
> 
>   Paste in the raw license list as a comment above License, and use it to
>   construct an SPDX expression. You can reorder terms, and you can
> deduplicate
>   equivalent terms. For example, (Apache-2.0 OR MIT) is equivalent to (MIT OR
>   Apache-2.0).
>   
>   Fabio Valentini (FAS: decathorpe) has a convention of (1) source license of
>   the package itself, then (2) single-term licenses in alphabetical order,
> then
>   (3) sub-expressions in alphabetical order, but without any reordering
> within
>   the parentheses – all with duplicates removed, e.g. (MIT OR Apache-2.0) AND
>   (Apache-2.0 OR MIT) can just be either (MIT OR Apache-2.0) or (Apache-2.0
> OR
>   MIT). This isn’t the only reasonable approach, and even just listing terms
>   without deduplication would be acceptable.
> 
>   The result could look something like this:
> 
>     # Apache-2.0
>     # Apache-2.0 OR MIT
>     # MIT
>     # MIT OR Apache-2.0
>     # Unlicense OR MIT
>     License:        %{shrink:
>         Apache-2.0 AND
>       MIT AND
>       (Apache-2.0 OR MIT) AND
>       (Unlicense OR MIT)
>         }
> 
>   As you maintain the package, it’s good to periodically check if the output
> of
>   %{cargo_license_summary} has changed, especially on major updates. It can
>   also sometimes change due to updates in your dependencies, even if your
>   package hasn’t changed.
> 

Thanks! I've added the %{cargo_license_summary} output to the specfile.

> 
> ===== Notes (no change required for approval) =====
> 
> - Consider using the description text for the RPM package description, as I
>   think it’s a bit more, well, descriptive.
> 
>   Since there are no subpackages re-using the same description text, there’s
> no
>   longer any need to wrap it up in an RPM macro.
> 
>   Consider something like this:
> 
>     %description
>     Tool to configure cached EIF files for the krun-awsnitro runtime for AWS
>     Nitro Enclaves.

Sure, I've updated the description.

> 
> - There’s no longer any need to explicitly number Sources unless it makes
>   easier for you to refer to multiple sources by number. Instead of 
> 
>     Source0:        […]
> 
>   you may just write:
> 
>     Source:         […]
> 
> - I think that the source URL
> 
>     %{url}/archive/refs/tags/v%{version}.tar.gz
> 
>   could be better written as
> 
>     %{url}/archive/v%{version}/%{name}-%{version}.tar.gz
> 
>   Then the archive name would match the extraction directory name.
> 

I've updated the spec file to reflect "Source" and the modified URL.

> - The Summary usually starts with a capital letter, and rpmlint does warn
> about
>   this:
> 
>     krun-awsnitro-eif-ctl.x86_64: W: summary-not-capitalized krun-awsnitro
> EIF configuration tool
> 
>   I don’t think there’s any formal guideline about this, and no change is
>   required. However, would something like this make sense?
> 
>     Summary:        EIF configuration tool for krun-awsnitro
> 

Yes, that makes sense. I've updated the Summary to reflect that.

Thanks for your feedback!


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2440262

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202440262%23c28

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]
Do not reply to spam, report it: 
https://forge.fedoraproject.org/infra/tickets/issues/new

Reply via email to