Bug 1268090
| Summary: | Review Request: python-requests-mock - Mock out responses from the requests package | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | William Moreno <williamjmorenor> |
| Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
| Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | christos.triantafyllidis, e, hobbes1069, package-review, rbarlow, williamjmorenor, zebob.m |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | BuildFails | ||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2018-03-16 19:59:42 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
William Moreno
2015-10-01 19:31:38 UTC
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. Hello, I'm afraid there are a few issues that block the review of this. Looks like the build based on that SRPM fails: http://koji.fedoraproject.org/koji/taskinfo?taskID=12043662 There are missing dependencies (at least buildrequires for python(3)-pbr). Also the source url seems to be wrong: http://pypi.python.org/packages/source/g/%{srcname}/%{srcname}-%{version}.tar.gz The correct one should be: http://pypi.python.org/packages/source/r/%{srcname}/%{srcname}-%{version}.tar.gz Please fix the above and remove "BuildFails" from the whiteboard to make it re-appear to the list. Cheers, Christos Any update? 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 > DEADREVIEW 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. True. |