Red Hat Bugzilla – Bug 1268090
Review Request: python-requests-mock - Mock out responses from the requests package
Last modified: 2018-03-16 15:59:42 EDT
Spec URL: https://williamjmorenor.fedorapeople.org/rpmdev/python-requests-mock.spec
SRPM URL: https://williamjmorenor.fedorapeople.org/rpmdev/python-requests-mock-0.6.0-1.fc22.src.rpm
Description: Mock out responses from the requests package
Fedora Account System Username: williamjmorenor
Rawhide Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=11297109
Hi William! I am also seeking to become a maintainer, so this review is informal only. Here are a few notes that I have for you:
0) It is conventional to name Python 2 packages with a python- prefix and Python 3 packages with a python3- prefix, but this package uses python2- instead of python. You will probably be required to fix that.
1) Is this package only intended to support EPEL >= 7 and Fedora? If so that is fine but if not you may need some macros to make sure the Python 3 blocks only execute on systems that support Python 3.
2) You've repeated the description three times. I believe it is possible to only write it once at the top, since the two packages share the same description. I believe this holds for the summary as well.
3) You've commented out the lines in your check section. You probably should include those lines unless the tests don't pass in the upstream project. If they do not pass, I recommend removing the commented code for cleanliness.
4) Sometimes the information provided in the egg.info files can be important for certain packages, particularly if Python entry points are used. You may want to leave that in instead of removing it, but it depends on the package.
Looks good, good luck!
(In reply to Randy Barlow from comment #1)
> 4) Sometimes the information provided in the egg.info files can be important
> for certain packages, particularly if Python entry points are used. You may
> want to leave that in instead of removing it, but it depends on the package.
Yes, unless there is some compelling reason to remove it, the Python packaging guidelines require that the egg.info be included in the package.
I'm afraid there are a few issues that block the review of this.
Looks like the build based on that SRPM fails:
There are missing dependencies (at least buildrequires for python(3)-pbr).
Also the source url seems to be wrong:
The correct one should be:
Please fix the above and remove "BuildFails" from the whiteboard to make it re-appear to the list.
Hello any uodate here? If not I will need to close this ticket as a DEADREVIEW
(In reply to William Moreno from comment #5)
> Hello any uodate here? If not I will need to close this ticket as a
Well that's your own package, if you bump the version to 1.4.0, I'll review it.
Also the summary and description looks wrong.