https://bugzilla.redhat.com/show_bug.cgi?id=2012522 --- Comment #7 from Mohamed El Morabity <pikachu.2014@xxxxxxxxx> --- (In reply to Maxwell G from comment #5) > > * 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? Do you think of python3dist(...)? This is the purpose of %py3_dist actually: $ rpm --eval "%{py3_dist my-nice-blob}" python3dist(my-nice-blob) > > * 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. Sounds legit. pytest is OK then if upstream uses it in its CI. > > (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 "%pyproject_buildrequires -t" is "%pyproject_buildrequires -r" + tox deps, I thought it would have been consistent to use tox also in %check. As long as BRs are satisfied this way, it's OK, there's nothing wrong :). > > * according to rpmlint, some Python files contain shebangs whereas they're not > > executable: > > I already fixed that in Comment #2, but I did it differently. Sorry, I made my review on a previous spec file. As long as unwanted shebangs are gone, it's OK too. (In reply to Maxwell G from comment #6) > Spec URL: > https://git.sr.ht/~gotmax23/fedora-package-yt-dlp/blob/f30a5d4f6d5134356f3dd96d0d80d1b9a96a6246/yt-dlp.spec 404 :'( -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. 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
