Bug 1914369
| Summary: | Re-Review Request: python-pyswip - Python/Prolog bridge | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Christoph Karl <pampelmuse> |
| Component: | Package Review | Assignee: | José Matos <jamatos> |
| Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | jamatos, package-review |
| Target Milestone: | --- | Flags: | jamatos:
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-06-26 05:21:11 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
Christoph Karl
2021-01-08 16:49:22 UTC
I am taking the review. The first thing to note is the SPEC url is not a spec file but an html page. :-) It should have been https://src.fedoraproject.org/fork/pampelmuse/rpms/python-pyswip/raw/master/f/python-pyswip.spec or else when running fedora-review fails. I will proceed the review using the srpm. The license is correct and appropriated for Fedora.
The spec file is simple and correct.
The only complaint of fedora-review is that the name is already taken, but this is OK since this is a re-review.
And so everything is fine and dandy. :-)
I have just one question, why do you have the following python_provide line?
%{?python_provide:%python_provide python3-%{srcname}}
The python_provides is obsolete, see: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_provides At most is should be replaced with py_provides and even so it is only relevant for Fedora 32 and unnecessary for Fedora 33+. Thank you very much for the review.
Sorry about the strange spec file.
I removed the line
%{?python_provide:%python_provide python3-%{srcname}}
It's unlikely that there will be a Fedora 32 package that depends on python3-pyswip before Fedora 32 is retired.
I also removed "-p1" from autosetup because there is currently no patch needed.
Result can be found here:
https://src.fedoraproject.org/fork/pampelmuse/rpms/python-pyswip/commits/master
Koji build here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=59249635
(In reply to Christoph Karl from comment #4) > Thank you very much for the review. > > Sorry about the strange spec file. The spec file is OK. My point is that the urls that you give should be the raw versions, just like you did for the srpm. :-) > I removed the line > %{?python_provide:%python_provide python3-%{srcname}} > > It's unlikely that there will be a Fedora 32 package that depends on > python3-pyswip before Fedora 32 is retired. OK. > I also removed "-p1" from autosetup because there is currently no patch > needed. I noticed that when reviewing the package. It could be there since that does not change the behavior of the package it is OK to let it IMHO. Certainly that package is clean without it. :-) I could cite the Zen of Python. :-) So this is fine. > Result can be found here: > https://src.fedoraproject.org/fork/pampelmuse/rpms/python-pyswip/commits/ > master > > Koji build here: > https://koji.fedoraproject.org/koji/taskinfo?taskID=59249635 Again the point is that you should have placed the raw links. I sometimes use helper scripts, just like fedora-review. That is why there is a template with the "Spec URL:" and "Spec URL:" lines in evidence. E.g. fedora-review will always pick the last version from there. This is more a matter of principle since in this case the change is minimal and in this case practical. The package is approved. Released 5 month ago. |