https://bugzilla.redhat.com/show_bug.cgi?id=1795077 Fabio Valentini <decathorpe@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |decathorpe@xxxxxxxxx Assignee|nobody@xxxxxxxxxxxxxxxxx |decathorpe@xxxxxxxxx Flags| |fedora-review? --- Comment #1 from Fabio Valentini <decathorpe@xxxxxxxxx> --- Hi, I'll review your package. Some recommendations: 1) I assume you're hiding tests behind the api_key bcond because they require an actual API key and internet access? So do you run tests locally with "rpmbuild -bb --with api_key"? Maybe add a comment for the bcond or in %check. 2) I recommend you use HTTPS for the URL as well. Then you can also replace the URL prefix of the Source0 with %{url}. Also, why are you not using %{pypi_source} directly? Do the pypi sources miss some files? If so, adding a comment why you're using the GitHub tarball instead would be helpful for anybody who's looking at the package. 3) You could deduplicate the %description, with something like: %global _description %{expand: The official Python library and CLI for Shodan Shodan is a search engine for Internet-connected devices. Google lets you search for websites, Shodan lets you search for devices. This library provides developers easy access to all of the data stored in Shodan in order to automate tasks and integrate into existing tools.} Then you can use: %description %{_description} and %description python3-%{pypi_name} %{_description} 4) Since you're shipping both python3-shodan and shodan binary packages, you could just name the source package shodan directly. But that's a matter of style, I guess. If you'll keep the source package named python-shodan, then "%files -n python-%{pypi_name}-doc" is superfluous and could be just "%files doc". 5) I guess you're removing a stray shebang line from worldmap.py in %prep? Adding a descriptive comment would be great. 6) For listing python3 modules, please use trailing slashes: %{python3_sitelib}/%{pypi_name}/ %{python3_sitelib}/%{pypi_name}-*.egg-info/ This will prevent upgrade issues if these ever change from directories to files. You could also use something like this instead of the second line: %{python3_sitelib}/%{pypi_name}-%{version}-py%{python3_version}.egg-info/ Then you'd not even need the glob. Let me know if these points are helpful, and when the package is ready for final review. -- 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 -- 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
