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



--- Comment #4 from Ben Rosser <[email protected]> ---
Thanks for taking this! Let me know what you'd like me to review in exchange.

> - About the name of the package, you have asserted that this is not a standard
  python library.  Yet I look at the tests and the examples, and I see
  "import cocotb".  I look at the final RPM, and I see a python module plus one
  binary, /usr/bin/cocotb-config, which certainly does not sound like the name
  of an application.  This sure looks like a Python module.  I think it should
  have the "python-" prefix.

Well... while it's true that the Python parts of the code can be imported as a
normal Python package, much of the code is only usable when running embedded
inside a RTL simulator, which is why I asserted it's not a standard Python
library.

I take your point that it's not really an "application", but I think there's
still some utility in letting users find it by the upstream name. How would you
feel about renaming the package to 'python-cocotb' but keeping 'cocotb' as a
provide?

> python3.8dist(setuptools)

Will remove. pyp2rpm still thinks it needs to manually list Python
dependencies-- I guess this probably should get updated upstream.

> These lines in %prep confuse me:

Also put in by pyp2rpm automatically, and I didn't bother to remove it. Happy
to get rid of it.

> For the change to the shebang in bin/combine_results.py, in addition to
  changing from unversioned python to python3, also get rid of /usr/bin/env,
  perhaps like this:

Will fix.

>  You might want to tell upstream about python 3.8 incompatibilities (seen in
  the build log):

Will pass these on.

> %{python3_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info

This is another pyp2rpm-generated line. It's probably worth getting this fixed
in pyp2rpm upstream too (certainly, I have other Python packages with this in
my %files list and I'm sure other people do too).

I will fix it here (I guess one simple fix is to change the second "?" to a
"*") though.

-- 
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
_______________________________________________
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]

Reply via email to