Bug 1392090
| Summary: | Review Request: pychromecast - Python library to communicate with the Google Chromecast | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Peter Robinson <pbrobinson> |
| Component: | Package Review | Assignee: | Athos Ribeiro <athoscribeiro> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | athoscribeiro, package-review |
| Target Milestone: | --- | Flags: | athoscribeiro:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-12-22 04:23:09 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: | 1209685, 1392089, 1401337 | ||
| Bug Blocks: | 1269538 | ||
|
Description
Peter Robinson
2016-11-04 19:33:41 UTC
I am not sure if we can use this name for the package. See https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#Outdated_Python_package_naming_conventions Would you use the template on https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file? It would really improve readability with all those macros set there. As pointed in https://fedoraproject.org/wiki/Packaging:Python#Reviewer_checklist , %python_provide macro must be used. are there any reasons for not including the README file under %doc? Any comments on why you decided to use github instead of pypi for sources? I am just curious since you used pypi for the other python package. (In reply to Athos Ribeiro from comment #1) > I am not sure if we can use this name for the package. See > https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging: > NamingGuidelines#Outdated_Python_package_naming_conventions The binary packages are called python2- and python3- so I don't see what the issue is. Updated, spec as before SRPM: https://pbrobinson.fedorapeople.org/pychromecast-0.7.7-2.fc25.src.rpm Please continue the review or should I get someone else to continue it? Hi Peter,
This is not a problem in this package, but I'd suggedt to be careful with wildcards in python3 packages, since sometimes your package may end up owning %{python3_sitelib}/__pycache__, which belongs to system-python-libs
spec file line 2 reads:
%define with_tests 0
Guidelines sugest we use %global instead, as you can see in [1]. Note that this is not a must.
The Summary tag ends with period a period. Please see [2].
After your feedback on those points I will consider this review done, sice the package looks good.
We would still need to find a solution for bug 1392089 in order to include python2-zeroconf in fedora, since this bug depends on it.
[1] https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
[2] https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections
> spec file line 2 reads: > %define with_tests 0 > > Guidelines sugest we use %global instead, as you can see in [1]. Note that > this is not a must. > > The Summary tag ends with period a period. Please see [2]. Those two are minor quirks I would fix on commit. Do you want me to update them? > After your feedback on those points I will consider this review done, sice > the package looks good. > > We would still need to find a solution for bug 1392089 in order to include > python2-zeroconf in fedora, since this bug depends on it. I'll deal with that. > Those two are minor quirks I would fix on commit. Do you want me to update > them? Not really, I trust you will address those. python2-zeroconf is still not in Fedora and bug 1392089 on which this bug depends on, might not solve the issue, as pointed out in that bug. I know you are an experienced packager and I do trust your judgement on how to solve the issue. The package looks good and this review is complete. Since all dependencies are not in Fedora yet, see [1] for reference. [1] https://fedoraproject.org/wiki/Packaging:ReviewGuidelines#A_note_on_dependencies Fixed those two points locally, requested maint on the other package Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/pychromecast Pushed. Global and summary fixed. Thanks for the review! |