https://bugzilla.redhat.com/show_bug.cgi?id=2278422 --- Comment #3 from wojnilowicz <lukasz.wojnilowicz@xxxxxxxxx> --- Once again a full list of my remarks: 1) Previous post from Fedora Review Service says it doesn't build. Could you fix that? I'm not sure if that's related, but it asked me three times about various missing dependencies when I tried to build it myself. 2) Could you use https://src.fedoraproject.org/rpms/python-jupytext/blob/rawhide/f/python-jupytext.spec#_52 https://src.fedoraproject.org/rpms/python-jupytext/blob/rawhide/f/python-jupytext.spec#_144 instead of "# Move the configuration files to where we want them"? 3) rpmlint reported python3-nbdime.noarch: W: no-manual-page-for-binary nbdime. Could you fix that? 4) rpmlint reported python3-nbdime.noarch: W: files-duplicate /usr/lib/python3.12/site-packages/nbdime/webapp/testnotebooks/base.ipynb /usr/lib/python3.12/site-packages/nbdime/tests/files/multi_cell_nb.ipynb Could you fix that? 5) Does it have any sense to list graphic files under the BSD license as in "_static/plus.png: BSD-2-Clause"? I think it's only for source code and at https://docs.fedoraproject.org/en-US/legal/allowed-licenses/#_allowed_content_licenses it isn't listed for graphic files. 6) Why giving executable bits to these files? Other py files don't have them. # Add missing executable bits chmod a+x \ %{buildroot}%{python3_sitelib}/nbdime/tests/filters/add_helper.py \ %{buildroot}%{python3_sitelib}/nbdime/tests/filters/noop.py \ %{buildroot}%{python3_sitelib}/nbdime/tests/filters/strip_outputs.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/git/diffdriver.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/git/difftool.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/git/mergedriver.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/git/mergetool.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/hg/diff.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/hg/diffweb.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/hg/merge.py \ %{buildroot}%{python3_sitelib}/nbdime/vcs/hg/mergeweb.py \ %{buildroot}%{python3_sitelib}/nbdime/webapp/nbdifftool.py \ %{buildroot}%{python3_sitelib}/nbdime/webapp/nbdiffweb.py \ %{buildroot}%{python3_sitelib}/nbdime/webapp/nbdimeserver.py \ %{buildroot}%{python3_sitelib}/nbdime/webapp/nbmergetool.py \ %{buildroot}%{python3_sitelib}/nbdime/webapp/nbmergeweb.py 7) No difference between following two entries intended? makehelp nbmerge 'Merge two Jupyter notebooks' makehelp nbmerge-web 'Merge two Jupyter notebooks' 8) Could you double check that you need it? The subsequent py3_shebang_fix removes "/usr/bin/env" as described at https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#py3_shebang_fix in my case. # Remove useless shebangs sed -i '\,#!/usr/bin/env,d' \ nbdime/diffing/directorydiff.py \ nbdime/gitfiles.py \ nbdime/webapp/nb_server_extension.py \ nbdime/webapp/webutil.py 9) How do you know which npms you have to bundle? There are many more of them in your vendored packages. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2278422 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202278422%23c3 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
