Bug 1667718
| Summary: | Review Request: python2-jsonschema - JSON Schema validation implementation for Python | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Fabio Valentini <decathorpe> |
| Component: | Package Review | Assignee: | Miro Hrončok <mhroncok> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | mhroncok, package-review |
| Target Milestone: | --- | Flags: | mhroncok:
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: | 2019-01-22 15:15:44 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
Fabio Valentini
2019-01-20 11:34:16 UTC
1. Note that there is also python2-backports-functools_lru_cache which IMHO makes more sense than using python2-repoze-lru. However both packages are still needed by stuff in Fedora so switching to one or another won't unblock the remaining one, so this is not important.
2. Are the RHEL6 conditionals worth it? Will the package even build on RHEL6? Are we planning to change anything in EPEL6 about this? I hope we are not.
3. The manual "Requires: python2-repoze-lru" is not needed, the patch patches setup.py and hence the package requires python2.7dist(repoze.lru).
4. Is %{_bindir}/jsonschema-2 needed? Can we not ship it and only consider this a compat library package?
5. I suggest to use %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info/ with the leading slash so no surprises happen if it happens to be a directory.
6. setup.py uses setuptools, BR python2-setuptools explicitly (the transitive dependency can go away in the future, see python2.spec for details).
Everything else looks good.
Note A: Depends on deprecated python2 but is a rename, so ti si allowed.
Note B: It is a rename, but no binary package rename, so no new provides/obsoletes are needed.
CHECKSUM(SHA256) this package : 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02
CHECKSUM(SHA256) upstream package : 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02
Rpmlint
-------
Checking: python2-jsonschema-2.6.0-7.fc30.noarch.rpm
python2-jsonschema-2.6.0-7.fc30.src.rpm
python2-jsonschema.noarch: W: spelling-error %description -l en_US validator -> lavatorial
python2-jsonschema.noarch: W: no-manual-page-for-binary jsonschema-2
python2-jsonschema.src: W: spelling-error %description -l en_US validator -> lavatorial
2 packages and 0 specfiles checked; 0 errors, 3 warnings.
Good.
5. ...if it happens to stop being a directory. (In reply to Miro Hrončok from comment #1) > 1. Note that there is also python2-backports-functools_lru_cache which IMHO > makes more sense than using python2-repoze-lru. However both packages are > still needed by stuff in Fedora so switching to one or another won't unblock > the remaining one, so this is not important. I didn't change this, it was introduced in the original python-jsonschema package. If either of them are going to be removed, we can switch it to use the other one. I don't care either way, but what's currently there is somewhat battle-tested. > 2. Are the RHEL6 conditionals worth it? Will the package even build on > RHEL6? Are we planning to change anything in EPEL6 about this? I hope we are > not. You're right, EPEL6 is definitely out of scope for this package, so I'll drop this. > 3. The manual "Requires: python2-repoze-lru" is not needed, the patch > patches setup.py and hence the package requires python2.7dist(repoze.lru). Right. > 4. Is %{_bindir}/jsonschema-2 needed? Can we not ship it and only consider > this a compat library package? I don't think anything depends on a python2 version of this script. I'll drop it. > 5. I suggest to use > %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info/ with the leading > slash so no surprises happen if it happens to stop being a directory. Done. > 6. setup.py uses setuptools, BR python2-setuptools explicitly (the > transitive dependency can go away in the future, see python2.spec for > details). Done. > Everything else looks good. > > Note A: Depends on deprecated python2 but is a rename, so it is allowed. > > Note B: It is a rename, but no binary package rename, so no new > provides/obsoletes are needed. I also bumped the Release, so it should be a clean upgrade over the existing python2 package from python-jsonschema. > CHECKSUM(SHA256) this package : > 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02 > CHECKSUM(SHA256) upstream package : > 6ff5f3180870836cae40f06fa10419f557208175f13ad7bc26caa77beb1f6e02 > > > Rpmlint > ------- > Checking: python2-jsonschema-2.6.0-7.fc30.noarch.rpm > python2-jsonschema-2.6.0-7.fc30.src.rpm > python2-jsonschema.noarch: W: spelling-error %description -l en_US validator > -> lavatorial > python2-jsonschema.noarch: W: no-manual-page-for-binary jsonschema-2 > python2-jsonschema.src: W: spelling-error %description -l en_US validator -> > lavatorial > 2 packages and 0 specfiles checked; 0 errors, 3 warnings. > > Good. I think I addressed all your comments. Spec URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema.spec SRPM URL: https://decathorpe.fedorapeople.org/packages/python2-jsonschema-2.6.0-8.fc29.src.rpm You have addressed all my comments. Package APPROVED. I suggest to have the python-jsonschema update ready before this is built in rawhide to hava two python2-jsonschemas for only a short period of time. Thanks! I've prepared the python-jsonschema update and sent off the PR. I'll wait with building this package until everything is push-button ready. (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python2-jsonschema I have successfully built this package for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=32192528 Thanks everybody! |