https://bugzilla.redhat.com/show_bug.cgi?id=2012522 --- Comment #5 from Maxwell G <redhat_bugzilla@xxxxxxxxxxxxxxxxxx> --- Hi Mohamed, Thank you for your suggestions. --- Comment #3 from Mohamed El Morabity <pikachu.2014@xxxxxxxxx> --- > * you should use the %py3_dist macro for explicit Python dependencies, for > example %{py3_dist keyring} instead of python3-keyring. This makes dependency > tracking easier, based on PyPi names. This may be useful also in case of naming > convention changes for Python packages. Alright, I will change that. Isn't there also another way to specify that where the module name is inside parentheses? > * Why not using %tox for tests in %check section, instead of an explicit call > to %pytest (and the corresponding BuildRequires)? I did not use Tox, because upstream's CI just calls pytest directly, and I didn't see the need to introduce the extra complexity of Tox. If you still think I should change it, I can. > (BTW you are > generating BRs with "%pyproject_buildrequires -t"). Yes, I am aware of that. I manually added pytest, because it wasn't already listed in the setup.py. Was that wrong? I will also fix that to use the macro, as you recommended. > * Since mutagen is already required by the package (automatically added as > Requires from setup.py), the Suggests on AtomicParsley is probably useless. You are right; the project README also says that they are redundant. > * according to rpmlint, some Python files contain shebangs whereas they're not > executable: > > yt-dlp.noarch: E: non-executable-script > /usr/lib/python3.9/site-packages/yt_dlp/YoutubeDL.py 644 /usr/bin/env python3 > yt-dlp.noarch: E: non-executable-script > /usr/lib/python3.9/site-packages/yt_dlp/__init__.py 644 /usr/bin/env python3 > yt-dlp.noarch: E: non-executable-script > /usr/lib/python3.9/site-packages/yt_dlp/__main__.py 644 /usr/bin/env python3 > yt-dlp.noarch: E: non-executable-script > /usr/lib/python3.9/site-packages/yt_dlp/utils.py 644 /usr/bin/env python3 > > You can use for example > https://fedoraproject.org/wiki/Packaging_tricks#Remove_shebang_from_Python_libraries > to fix this, while keeping file timestamp. I already fixed that in Comment #2, but I did it differently. > # See https://github.com/yt-dlp/yt-dlp/commit/5e1dba8ed6a8974405ed038cb1ed7a82cdfaca4b#commitcomment-57786462 > find -type f ! -executable -name '*.py' -print -exec sed -i -e '1{\@^#!.*@d}' '{}' + I think this solution is more elegant, because it only applies to non-executable files. It doesn't preserve the timestamp, though. --- Comment #3 from Mohamed El Morabity <pikachu.2014@xxxxxxxxx> --- > Another rpmlint issue: > > yt-dlp.src:72: W: rpm-buildroot-usage %build make DESTDIR=%{buildroot} README.txt yt-dlp.1 completion-bash completion-zsh completion-fish > > You should remove the DESTDIR variable in this make call. Setting this variable is useless since building these extra files doesn't rely on target installation directory. I will remove that. Thanks again, Maxwell -- 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=2012522 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
