Bug 1910141
| Summary: | Review Request: rubygem-webrick - HTTP server toolkit | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Pavel Valena <pvalena> |
| Component: | Package Review | Assignee: | Jaroslav Prokop <jar.prokop> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | jar.prokop, package-review, vondruch |
| Target Milestone: | --- | Flags: | jar.prokop:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2021-01-06 20:08:32 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: | |||
|
Description
Pavel Valena
2020-12-22 19:53:11 UTC
I'll take this for a review. Licensing:
* License: BSD-2-Clause [0] should be just BSD [1] according to licensing guidelines. Maybe just add a note above specifying that it is BSD-2-Clause?
* Is the `Ruby license` specified in the `License:` field required? (If so, why? I am curious a bit myself)
One small nit:
* The `%{gem_instdir}/bin` could be in my opinion excluded. It adds nothing meaningful for users as they are there just for development purposes.
The package builds and works so it looks good to go after clarifying the licensing.
[0] https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#2ClauseBSD
[1] https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses
Thank you for your quick review! TBH I've not checked the package much, as it's needed for Ruby 3.0 rebuilds, which I'm focused on now. (In reply to Jaroslav Prokop from comment #2) > Licensing: > * License: BSD-2-Clause [0] should be just BSD [1] according to licensing > guidelines. Maybe just add a note above specifying that it is BSD-2-Clause? Yes, you're right, I'll fix it to `BSD`. > * Is the `Ruby license` specified in the `License:` field required? (If so, > why? I am curious a bit myself) `Ruby` is a valid license, but it's not in the repo itself, so I've created an issue: https://github.com/ruby/webrick/issues/64 The response was Ruby is the correct License as well. I'll keep both then. > > One small nit: > * The `%{gem_instdir}/bin` could be in my opinion excluded. It adds nothing > meaningful for users as they are there just for development purposes. Good catch! I'll move it into doc subpackage. > > The package builds and works so it looks good to go after clarifying the > licensing. > > [0] https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#2ClauseBSD > [1] > https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses Here's the fixed spec and SRPM: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01847159-rubygem-webrick/rubygem-webrick.spec https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01847159-rubygem-webrick/rubygem-webrick-1.7.0-1.fc34.src.rpm And the diff: https://git.io/JLPQv (I'll re-add the Ruby license, as per the upstream response.) > > * Is the `Ruby license` specified in the `License:` field required? (If so, > > why? I am curious a bit myself) > > `Ruby` is a valid license, but it's not in the repo itself, so I've created > an issue: > https://github.com/ruby/webrick/issues/64 > > The response was Ruby is the correct License as well. I'll keep both then. > (I'll re-add the Ruby license, as per the upstream response.) Nice, with that concluded, I see nothing else that needs changing. Package approved! (I see I forgot to assign the bug to me, I'll do so now, better later than never.) s/otu/out/ (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rubygem-webrick |