https://bugzilla.redhat.com/show_bug.cgi?id=1795077
--- Comment #3 from Fabio Valentini <decathorpe(a)gmail.com> ---
(In reply to Fabian Affolter from comment #2)
%{url} could be an option. If the reviewer wants that then I will add
it.
Otherwise I prefer to keep things "copy-and-pastable" for humans in the spec
file.
Right, but since non-copy-pastable %{version} and %{name} are already in there,
using %{url} won't make it worse :)
> 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.
GitHub is the upstream location for the source and not PyPI. As a long as
the project is providing proper tarballs I think that we should stick to the
original upstream location and not a third-party delivery mechanism.
That's fair, and your decision.
> 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}
Again, this removes the possibility to re-use the description outside of the
spec file with copy-and-paste. It makes sense if you have a dozen of
subpackages and want to use the same text but for regular packages it
doesn't add much value from my point of view.
How so? It just moves the definition a few lines up in the .spec file ...
> 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".
From my point of view, it's at the packager's discretion. This comes down to
the decision if it is more a tool or a lib. In most cases the CLI part is an
add-on to the lib.
Correct.
Still, "%files -n python-%{pypi_name}-doc" is superfluous and could be replaced
with "%files doc".
> 5) I guess you're removing a stray shebang line from
worldmap.py in %prep?
> Adding a descriptive comment would be great.
I really think that we should not add comments to every standard/common
building block like remove shebang, fix permissions, remove the egg, remove
the left-overs from the docs generation, etc.
https://github.com/achillean/shodan-python/pull/118
That's your decision, it was only a suggestion.
Not everybody can sight-read sed scripts ;)
> 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.
Fixed.
Great.
> Let me know if these points are helpful, and when the package is
ready for
> final review.
Thank you for your feedback.
* Tue Jan 28 2020 Fabian Affolter <mail(a)fabian-affolter.ch> - 1.21.3-2
- Fix ownership
- Improve the check workflow (rhbz#1795077)
Updated files:
Spec URL:
https://fab.fedorapeople.org/packages/SRPMS/python-shodan.spec
SRPM URL:
https://fab.fedorapeople.org/packages/SRPMS/python-shodan-1.21.3-2.fc31.src.
rpm
I'll run fedora-review for the formal review now.
--
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