https://bugzilla.redhat.com/show_bug.cgi?id=2114603



--- Comment #9 from Jonathan Wright <[email protected]> ---
(In reply to Miro HronĨok from comment #8)
> > The spec file indeed looks a ton cleaner now.
> 
> Awesome, indeed it does.
> 
> 
> 
> A few more tiny nit-picks:
> 
> -----------
> 
> > BuildRequires:  python3-setuptools
> 
> vs.
> 
> > BuildRequires:  python3dist(pytest)
> 
> This is not consistent. Let's use python3dist(setuptools) and
> python3dist(pytest), or python3-setuptools and python3-pytest?

You know, I never would've typed the python3dist version if I did it myself. 
rpmdev-newspec is the gift that keeps on giving.

> -----------
> 
> > install -m 644 %{SOURCE1} LICENSE
> 
> This is part of %install, but it does not install the LICENSE file to the
> package, it merely copies the file to $PWD. Semantically, this belongs to
> %prep. I'd probably use `cp -a` instead of `install` myself, but that's just
> my personal opinion.

Moved to prep using cp.

> -----------
> 
> > %dir %{python3_sitelib}/kgb
> > %{python3_sitelib}/kgb/*
> 
> This does not look like a namespace package. I'd use simpler:
> 
> %{python3_sitelib}/kgb/

Indeed it works properly with the trailing / there.  It didn't without it.

> -----------
> 
> > %{python3_sitelib}/kgb-%{version}.dist-info*
> 
> Does this work:
> 
> %{python3_sitelib}/kgb-%{version}.dist-info/
> 
> ?
> 
> It is more explicit. Also easy to spot that it is a directory.

Yes indeed.

> -----------
> 
> 
> Approximatelly half of the pacakge is occupied by
> /usr/lib/python3.10/site-packages/kgb/tests/ -- consider either not
> packaging those at all, or moving them to a python3-kgb-tests subpackage.

I think I'd rather not package them versus a sub-package though all other
python specs I've looked at they just leave them in the main package.  I
checked a few popular python libs such as python3-pandas, python-urllib3, and
python3-numpy  A quick `dnf search test` didn't yield any real precedent of
sub-packages for tests nor could I find docs on it.

Can we just leave them to be in line with other packages?


-- 
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=2114603
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to