https://bugzilla.redhat.com/show_bug.cgi?id=2012522
--- Comment #7 from Mohamed El Morabity <pikachu.2014(a)gmail.com> ---
(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)
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