https://bugzilla.redhat.com/show_bug.cgi?id=1795077
Fabio Valentini <decatho...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |decatho...@gmail.com
Assignee|nob...@fedoraproject.org |decatho...@gmail.com
Flags| |fedora-review?
--- Comment #1 from Fabio Valentini <decatho...@gmail.com> ---
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
