Bug 2116087
Summary: | Review Request: rust-strength_reduce - Faster integer division and modulus operations | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Orion Poplawski <orion> |
Component: | Package Review | Assignee: | Fabio Valentini <decathorpe> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | decathorpe, package-review |
Target Milestone: | --- | Flags: | decathorpe:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | rust-strength_reduce-0.2.3-1.fc38 | Doc Type: | If docs needed, set a value |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2022-11-03 02:48:16 UTC | Type: | Bug |
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: | 2116088, 2116092 |
Description
Orion Poplawski
2022-08-06 22:29:22 UTC
> Source1: http://www.apache.org/licenses/LICENSE-2.0 > Source2: http://opensource.org/licenses/MIT These are both wrong - they point at HTML pages, not at raw text files. Additionally, the link for MIT doesn't resolve, because that site is now HTTPS only. You should instead file a PR with the upstream project to add the license files (not HTML dumps :), and then you can link the files from your PR. You can look at how must Rust crates (for example, https://github.com/seanmonstar/reqwest) handle this for "inspiration". For example, the "LICENSE-APACHE" and "LICENSE-MIT" file names are pretty standardized across the whole ecosystem. > # Update version deps > # https://github.com/ejmahler/strength_reduce/pull/6 > Patch: rust-strength_reduce-deps.patch You might be interested in rust2rpm's "-p" flag, which automates generation of patches like this one. Please also include a link to the PR that you filed upstream for this. > %files devel > %license LICENSE-2.0 MIT This should be: %files devel %license %{crate_instdir}/LICENSE-APACHE %license %{crate_instdir}/LICENSE-MIT To match what rust2rpm generates if these files are present. It also looks like you uploaded an SRPM file that was already mangled by rpmautospec: > %changelog > * Sat Aug 06 2022 John Doe <packager> 0.2.3-1 > - Uncommitted changes I recommend to use "rpmbuild -bs" to build SRPM files for package review. "fedpkg srpm" will do rpmautospec pre-processing, which is not what you want for the package review. (In reply to Fabio Valentini from comment #1) > > Source1: http://www.apache.org/licenses/LICENSE-2.0 > > Source2: http://opensource.org/licenses/MIT > > These are both wrong - they point at HTML pages, not at raw text files. > Additionally, the link for MIT doesn't resolve, because that site is now > HTTPS only. > > You should instead file a PR with the upstream project to add the license > files (not HTML dumps :), and then you can link the files from your PR. > You can look at how must Rust crates (for example, > https://github.com/seanmonstar/reqwest) handle this for "inspiration". > For example, the "LICENSE-APACHE" and "LICENSE-MIT" file names are pretty > standardized across the whole ecosystem. Done. > > # Update version deps > > # https://github.com/ejmahler/strength_reduce/pull/6 > > Patch: rust-strength_reduce-deps.patch > > You might be interested in rust2rpm's "-p" flag, which automates generation > of patches like this one. Good to know, thanks. > Please also include a link to the PR that you filed upstream for this. Already there. > It also looks like you uploaded an SRPM file that was already mangled by > rpmautospec: > > > %changelog > > * Sat Aug 06 2022 John Doe <packager> 0.2.3-1 > > - Uncommitted changes Mainly just not having set %packager I think. > Mainly just not having set %packager I think.
No, that's a different problem.
Thanks for the other fixes, re-running checks now ...
Looks like you copy-pasted MIT license with a copyright statement from a different project / a different author for your license file PR? Hmm, indeed - didn't realize the copyright holder was mentioned in the license files. I've updated to be similar to one of his other projects. Hard to say if that is appropriate though. Spec URL: https://orion.fedorapeople.org/rust-strength_reduce.spec SRPM URL: https://orion.fedorapeople.org/rust-strength_reduce-0.2.3-1.fc38.src.rpm In this case, I'd rather like to be on the safe side, and wait for upstream response, before we ship license files that might not match what they want or intend. Unfortunately I'm getting no response from upstream. That's unfortunate. However, as far as I can tell, you included standard license texts for the licenses that were intended by upstream (since the MIT and Apache-2.0 SPDX identifiers are unique), so I'm not going to block on this if upstream is non-responsive. === 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 files are included with %license in %files (manually included from proposed upstream change) - 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 (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-strength_reduce Checked in and built. Thanks all. |