Bug 1727491 (python-lfpy)
Summary: | Review Request: python-lfpy - A module for modeling extracellular potentials of multicompartment neuron models built on NEURON | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Victor Tejada Yau <victortyau> |
Component: | Package Review | Assignee: | Luis Bazan <bazanluis20> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | bazanluis20, decathorpe, luis, mhroncok, package-review, qader.aymen, sanjay.ankur, zbyszek |
Target Milestone: | --- | Flags: | bazanluis20:
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-09-09 02:21:55 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: | |||
Bug Depends On: | |||
Bug Blocks: | 177841, 1276941 |
Description
Victor Tejada Yau
2019-07-07 00:26:55 UTC
Package name should be python-lfpy. Please don't repeat the description text: %description LFPy is a Python-module for calculation of extracellular potentials from ... have a model... ↓ %global %_description %{expand: LFPy is a Python-module for calculation of extracellular potentials from ... have a model...} %description %_description %description -n python3-%{pypi_name} %_description Spec URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy.spec SRPM URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy-2.0.2-1.fc30.src.rpm thanks a lot Hmm, now there are two description texts. Is this on purpose? Spec URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy.spec SRPM URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy-2.0.2-1.fc30.src.rpm thanks a lot Hi, I'm not sponsored yet so these are just a few unofficial comments (not a comprehensive review): * Your summary is too long -- it shouldn't exceed 79 characters. * The package doesn't build with mock, you need to list a BuildRequires against gcc (for cython). Try make sure that the package builds using mock -- check out https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds * Move the python3 specific BuildRequires statements to under the python3- subpackage. * Make sure that you are not using 'Requires' for dependencies which are only needed during the build step (e.g. setuptools is likely not required outside of building, so it should just be a BuildRequires). Likewise, BuildRequires shouldn't be used for dependencies which are not needed for building but are needed for the package to function. * Upstream provides examples, which you can include with %doc. They also provide sphinx docs, which you can build with sphinx-build and likewise include with %doc. All good comments from Aymen, with one exception:
> Move the python3 specific BuildRequires statements to under the python3- subpackage.
Nah, not this. It is normal to keep all BRs at the top. It only makes sense to split
them out when there are optional subpackage and one wants to make clear which BRs are
required for which subpackages. In this case, the python3- subpackage is "the" subpackage,
that always needs to be built and the BRs for that should be at the top.
>Nah, not this. It is normal to keep all BRs at the top. Apologies for the misinformation, I had thought that this was the convention because of the example spec in the guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_python_spec_file) I'll make sure to keep this in mind for the future, thank you for the correction. Victor, is this ready for a full review now? Luis, would you have the cycles to do it this week please? I can take it up otherwise since you're all going to be busy at Flock :P Cheers! Spec URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy.spec SRPM URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy-2.0.2-1.fc30.src.rpm some changes waiting for comments. Bless Hi Victor Please add on BuildRequires: gcc and remove on Requires setuptools and Cython and I can approve your package and them you add the %doc in spec Cheers, Hi Luis Spec URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy.spec SRPM URL: https://victortyau.fedorapeople.org/python-LFPy/python-lfpy-2.0.3-1.fc30.src.rpm some changes waiting for comments. Bless Hi Victor APPROVED! Please continue with the next step. Add the neuro-sig in members. :-) Cheers, (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-lfpy Why is the source package called python-lfpy and the subpacakge python3-LFPy instead of python3-lfpy? Victor, please fix the sub-package as Miro pointed out, and then also build for F31 and F30 and push updates to bodhi. FEDORA-2019-6f15ed9d48 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-6f15ed9d48 FEDORA-2019-8d739f7da7 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-8d739f7da7 (In reply to Miro Hrončok from comment #14) > Why is the source package called python-lfpy and the subpacakge python3-LFPy > instead of python3-lfpy? Fixed! Cheers, python-lfpy-2.0.3-5.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-53e7847129 python-lfpy-2.0.3-4.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-8d739f7da7 I don't know why you think you need to define a whole lot of custom macros, but at any rate the python3 sub-package is now named python3--lfpy with two hyphens. (In reply to Fabio Valentini from comment #22) > I don't know why you think you need to define a whole lot of custom macros, > but at any rate the python3 sub-package is now named python3--lfpy with two > hyphens. fixed! only in master and there is no upgrade path at all. FEDORA-2019-5075fa927b has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-5075fa927b (In reply to Miro Hrončok from comment #24) > only in master and there is no upgrade path at all. https://src.fedoraproject.org/rpms/python-lfpy/c/d3cc39fc0bd44d3d7c3e7759e0e76f9c96c3d66d?branch=f30 https://src.fedoraproject.org/rpms/python-lfpy/c/d3cc39fc0bd44d3d7c3e7759e0e76f9c96c3d66d?branch=f29 python-lfpy-2.0.3-6.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-9a55c5aed6 python-lfpy-2.0.3-6.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-5075fa927b python-lfpy-2.0.3-6.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report. python-lfpy-2.0.3-6.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report. The needinfo request[s] on this closed bug have been removed as they have been unresolved for 1000 days |