https://bugzilla.redhat.com/show_bug.cgi?id=2278420 --- Comment #9 from Jerry James <loganjerry@xxxxxxxxx> --- You ask great questions! (In reply to wojnilowicz from comment #8) > Ok. Could you then change the following comment "# Move the configuration > files to where we want them" to something mentioning that you move it to > match the python-jupyter-filesystem directory? I have added two comments, one above the Requires and one above the mv command. > Anyway I believe the jupytext-%{version}-vendor-licenses.txt doesn't match > the "License:" field as in > "[ ]: License field in the package spec file matches the actual license." > The txt file contains among other "BlueOak-1.0.0" and the spec file does > not. How should we handle it? The guidelines seem to assume that any node.js module needed as a BuildRequires will also be needed at runtime; i.e., it will be bundled. That may be true for node.js packages, but this is a Python package that uses some JavaScript at buildtime. The generated file of license names lists the licenses of the JavaScript installed at build time. The License field in the spec file describes the licenses of the files that are actually installed, as described in https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_field. > Why don't you use nodejs-packaging-bundler from > https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/ > #_bundling_script ? > It produces much smaller dependencies package of 38,6 MB vs yours 102,9 MB > with licenses listing included. Additional advantage is that you could drop > your custom script. Maybe then a dependency on npm could be dropped as well > because the structure of the archive is a bit different as well. It's smaller because it is missing all of the jupyterlab modules. I'm afraid using jlpm is necessary to construct a tarball that will work for building. > Ok. Shouldn't then jupytext-%{version}-vendor-licenses.txt contain only MIT > and BSD-3-Clause? It describes the licenses of the modules in the vendor tarball, as described above. That is not the same as the modules that are actually installed. > 8) You use pytest and "BuildRequires: python3dist(pytest)" is missing as > explained at > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > #_test_dependencies_2 The first part of that section describes using %pyproject_buildrequires in a %generate_buildrequires script. That is what this package does. > 9) Could you "help2man --no-discard-stderr" on jupytext-config to fix > "python3-jupytext.noarch: W: no-manual-page-for-binary jupytext-config" ? Sure. I didn't think that was useful, given the nature of jupytext-config, but I've gone ahead and done it. > 10) Out of curiosity, how did you prepare the breakdown of licenses for > "%package doc"? Awhile ago, I did a license analysis of various documentation generators. The results are here: https://jamezone.org/pleasure/software/Fedora/license/. Now when I want to analyze a doc package, I match up the files in the package with the list at that URL to generate the list of licenses. > Thanks for supporting me in this package review. No problem. You're doing a great job as a reviewer. I have uploaded a new spec file and a new srpm, at the same URLs as before. -- 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=2278420 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202278420%23c9 -- _______________________________________________ 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