https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Bug ID: 1392089 Summary: Review Request: python-zeroconf: Pure Python Multicast DNS Service Discovery Library Product: Fedora Version: rawhide Component: Package Review Assignee: nobody@fedoraproject.org Reporter: pbrobinson@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Blocks: 1269538 (IoT)
SPEC: https://pbrobinson.fedorapeople.org/python-zeroconf.spec SRPM: https://pbrobinson.fedorapeople.org/python-zeroconf-0.17.6-1.fc25.src.rpm
Description: Pure Python Multicast DNS Service Discovery Library
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=16290711
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1269538 [Bug 1269538] Tracker for IoT on Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Peter Robinson pbrobinson@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1392090
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1392090 [Bug 1392090] Review Request: pychromecast: Python library to communicate with the Google Chromecast.
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |athoscribeiro@gmail.com Assignee|nobody@fedoraproject.org |athoscribeiro@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #1 from Athos Ribeiro athoscribeiro@gmail.com --- 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 running upstream's test suite under %check? I see there's one here https://github.com/jstasiak/python-zeroconf/blob/master/test_zeroconf.py
Also, are there any reasons for not including the README file under %doc?
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #2 from Athos Ribeiro athoscribeiro@gmail.com --- There are also some missing Requires:
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mail@fabian-affolter.ch
--- Comment #3 from Athos Ribeiro athoscribeiro@gmail.com --- *** Bug 1395341 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #4 from Peter Robinson pbrobinson@gmail.com --- (In reply to Athos Ribeiro from comment #2)
There are also some missing Requires:
Feel free to actually list them explicitly, I'll get back to this next week, I've been dealing with f25 release so that's taken priority.
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #5 from Peter Robinson pbrobinson@gmail.com --- SPEC: https://pbrobinson.fedorapeople.org/python-zeroconf.spec SRPM: https://pbrobinson.fedorapeople.org/python-zeroconf-0.17.6-2.fc25.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=16708202
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Peter Robinson pbrobinson@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(athoscribeiro@gma | |il.com)
--- Comment #6 from Peter Robinson pbrobinson@gmail.com --- Please continue the review or should I get someone else to continue it?
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(athoscribeiro@gma | |il.com) |
--- Comment #7 from Athos Ribeiro athoscribeiro@gmail.com --- Hi Peter,
I am sorry for the delay here, I have been travelling in the past few weeks and probably missed the emails on this bug.
Please, see bug 1401337 and [1]
python-zeroconf was reviewed and approved during our review here, but the maintainer only included a python3-zeroconf package and we do need a python2-zeroconf for bug 1392090
We can either check if the maintainer is willing to add python2-zeroconf to the python-zeroconf package or rename this package to python2-zeroconf, remove the python3-zeroconf subpackage and proceed with the review.
I believe the former would be preferred, and if you agree, I would even contact the maintainer and send him a patch to include python2-zeroconf. Else, here is the review of the package:
I only found 2 issues here:
1 - the python3-zeroconf owns /usr/lib/python3.5/site-packages/__pycache__ which belongs to system-python-libs. This issue appears in a few python3 packages when we use wildcards like %{python3_sitelib}/*
Since the python3 subpackage would be removed, this should be ignored.
2 - spec file line 2 reads: %define with_tests 0
Guidelines sugest we use %global instead, as you can see in [2]. Note that this is not a must.
Other than that, the python2 package would be ready, if that's how you'd like to proceed.
[1] https://admin.fedoraproject.org/pkgdb/package/rpms/python-zeroconf/ [2] https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over...
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #8 from Peter Robinson pbrobinson@gmail.com ---
Please, see bug 1401337 and [1]
python-zeroconf was reviewed and approved during our review here, but the maintainer only included a python3-zeroconf package and we do need a python2-zeroconf for bug 1392090
That bug was submitted after this one, this one should have taken precidence.
We can either check if the maintainer is willing to add python2-zeroconf to the python-zeroconf package or rename this package to python2-zeroconf, remove the python3-zeroconf subpackage and proceed with the review.
No it needs to be added to the other one. GRRRRRR
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
--- Comment #9 from Athos Ribeiro athoscribeiro@gmail.com --- (In reply to Peter Robinson from comment #8)
Please, see bug 1401337 and [1]
That bug was submitted after this one, this one should have taken precidence.
Yes, they probably forgot to check bugzilla for a review request before submitting it... maybe we should have a bot checking for duplicated review requests.
We can either check if the maintainer is willing to add python2-zeroconf to the python-zeroconf package or rename this package to python2-zeroconf, remove the python3-zeroconf subpackage and proceed with the review.
No it needs to be added to the other one. GRRRRRR
OK. You said (in the other review request) you would address this, so I suppose I do not need to contact the maintainer to include a python2-zeroconf subpackage.
https://bugzilla.redhat.com/show_bug.cgi?id=1392089
Peter Robinson pbrobinson@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |DUPLICATE Last Closed| |2016-12-20 23:22:24
--- Comment #10 from Peter Robinson pbrobinson@gmail.com ---
OK. You said (in the other review request) you would address this, so I suppose I do not need to contact the maintainer to include a python2-zeroconf subpackage.
I will, I've requested co-maintainer, if they don't respond in a reasonable time I'll just push my needed changes anyway.
Marking as duplicate even though it should be the other way around.
*** This bug has been marked as a duplicate of bug 1401337 ***
package-review@lists.fedoraproject.org