https://bugzilla.redhat.com/show_bug.cgi?id=1795077 --- Comment #3 from Fabio Valentini <decathorpe@xxxxxxxxx> --- (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@xxxxxxxxxxxxxxxxxx> - 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 _______________________________________________ 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
