spec: https://fed500.fedorapeople.org/python-pytest-notebook.spec srpm: https://fed500.fedorapeople.org/python-pytest-notebook-0.10.0-1.fc41~bootstrap.src.rpm description: The purpose of the plugin is to ensure that changes to the python environment (e.g. updating packages), have not affected the outputs of the notebook. If the notebook has changed, this plugin can also regenerate the notebooks, saving the new outputs. fas: fed500 Reproducible: Always
Copr build: https://copr.fedorainfracloud.org/coprs/build/9081475 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368625-python-pytest-notebook/fedora-rawhide-x86_64/09081475-python-pytest-notebook/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
- About test requirements, could you eliminate the `black`, `beautifulsoup4`, `coverage` tests and dependencies. Most likely these tests would not be run downstream either, and I am not sure how much you want to burden yourself with maintaining that part. - About documentation, I believe the install location is not distro friendly? Could you move them to `%_pkgdocdir`, but also - Do you really need/want to package the documentations. Especially for a project like this that would not have a user-facing audience - The fact that `test_nb_regression` is failing, is quite telling. I think it would be cleaner to just ignore them altogether. Btw, nice bash-foo syntax on that - I know upstream has quite a lot on his plate, but if you could coordinate and merge (as in squash) some of the open PRs, that would make it easier for him to catch up with this - Could you add `README.md` to `%doc`? I am not aware of it being added automatically from `%pyproject_wheel` (though would be nice if it was) - The "Remove coverage from tests" could you make them a patch instead? `sed` would not catch if the original source has changed, but the latter would.
(In reply to Cristian Le from comment #2) > - About test requirements, could you eliminate the `black`, > `beautifulsoup4`, `coverage` tests and dependencies. Most likely these tests > would not be run downstream either, and I am not sure how much you want to > burden yourself with maintaining that part. Done > - About documentation, I believe the install location is not distro > friendly? Could you move them to `%_pkgdocdir`, but also This is mostly for README, AUTHORS, Changelog and other similar files. Documentation files for yelp go in %{_datadir}/help/ and then the appropriate language folder. Checking with Rüdiger Landmann who know about DocBook from maintaining Publican https://src.fedoraproject.org/rpms/publican Though may also need to ask on list. > - Do you really need/want to package the documentations. Especially for a > project like this that would not have a user-facing audience Helpful. Sometimes one is offline and wants documentation. > - The fact that `test_nb_regression` is failing, is quite telling. I think > it would be cleaner to just ignore them altogether. Helps verify functionality. > Btw, nice bash-foo > syntax on that Not mine. Stolen from Ben Beasley. > - I know upstream has quite a lot on his plate, but if you could coordinate > and merge (as in squash) some of the open PRs, that would make it easier for > him to catch up with this Not a good fit for me. Rarely use Python notebooks. If there are other things in HPC/Materials Science that could be useful, happy to help with those. Can also update pyproject.toml > - Could you add `README.md` to `%doc`? I am not aware of it being added > automatically from `%pyproject_wheel` (though would be nice if it was) Done > - The "Remove coverage from tests" could you make them a patch instead? > `sed` would not catch if the original source has changed, but the latter > would. Done spec: https://fed500.fedorapeople.org/python-pytest-notebook.spec srpm: https://fed500.fedorapeople.org/python-pytest-notebook-0.10.0-1.fc41~bootstrap.src.rpm
Created attachment 2091720 [details] The .spec file difference from Copr build 9081475 to 9092728
Copr build: https://copr.fedorainfracloud.org/coprs/build/9092728 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368625-python-pytest-notebook/fedora-rawhide-x86_64/09092728-python-pytest-notebook/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Benson Muite from comment #3) > (In reply to Cristian Le from comment #2) > > - Do you really need/want to package the documentations. Especially for a > > project like this that would not have a user-facing audience > > Helpful. Sometimes one is offline and wants documentation. Yes, but can it be accessed other than manually navigating to the index.html file? I've heard that KHelpCenter might do some magic, but I think it doesn't read `/usr/share/doc/<pkg>` format. Anyway, at the very least let's avoid the bootstrap. You should be able to export a `PYTHONPATH` if the script requires it to be built first [1] > > - The fact that `test_nb_regression` is failing, is quite telling. I think > > it would be cleaner to just ignore them altogether. > > Helps verify functionality. Up to you on this. Are there PRs that fix this test? > > - I know upstream has quite a lot on his plate, but if you could coordinate > > and merge (as in squash) some of the open PRs, that would make it easier for > > him to catch up with this > > Not a good fit for me. Rarely use Python notebooks. If there are other > things > in HPC/Materials Science that could be useful, happy to help with those. Can > also update pyproject.toml Ok, if I remember, I will give it a shot. [1]: https://sourcegraph.com/search?q=context:global+file:%5C.spec%24+PYTHONPATH&patternType=keyword&sm=0