[Bug 2116087] New: Review Request: rust-strength_reduce - Faster integer division and modulus operations
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Bug ID: 2116087 Summary: Review Request: rust-strength_reduce - Faster integer division and modulus operations Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: orion@nwra.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://orion.fedorapeople.org/rust-strength_reduce.spec SRPM URL: https://orion.fedorapeople.org/rust-strength_reduce-0.2.3-1.fc37.src.rpm Description: Faster integer division and modulus operations.
Fedora Account System Username: orion
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=90548736
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Orion Poplawski orion@nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2116088
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2116088 [Bug 2116088] Review Request: rust-transpose - Utility for transposing multi-dimensional data
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Orion Poplawski orion@nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2116092
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2116092 [Bug 2116092] Review Request: rust-rustfft - High-performance FFT library written in pure Rust
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Doc Type|--- |If docs needed, set a value Assignee|nobody@fedoraproject.org |decathorpe@gmail.com CC| |decathorpe@gmail.com
--- Comment #1 from Fabio Valentini decathorpe@gmail.com ---
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@example.com 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #2 from Orion Poplawski orion@nwra.com --- (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@example.com 0.2.3-1
- Uncommitted changes
Mainly just not having set %packager I think.
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- Looks like you copy-pasted MIT license with a copyright statement from a different project / a different author for your license file PR?
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #5 from Orion Poplawski orion@nwra.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #7 from Orion Poplawski orion@nwra.com --- Unfortunately I'm getting no response from upstream.
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
--- Comment #9 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-strength_reduce
https://bugzilla.redhat.com/show_bug.cgi?id=2116087
Orion Poplawski orion@nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |RAWHIDE Status|POST |CLOSED Fixed In Version| |rust-strength_reduce-0.2.3- | |1.fc38 Last Closed| |2022-11-03 02:48:16
--- Comment #10 from Orion Poplawski orion@nwra.com --- Checked in and built. Thanks all.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com