https://bugzilla.redhat.com/show_bug.cgi?id=2117112
--- Comment #5 from Jonathan Wright <jonathan(a)almalinux.org> ---
(In reply to Miro Hrončok from comment #4)
Further feedback, as I was the original reviewer for bz2090791 but
then got
cut off by Nick without involving me here :(
I started on the package before realizing he had one pending review. We
chatted on IRC and I offered him my spec but he said just submit it and he'd
review. Sorry about that.
> rm -rf src/%{name}.egg-info
This is most likely not needed and should not be in the spec file just
because some older packages have it.
Removed. Came across the specific docs on that today actually.
> Source0: %{pypi_source}
Using %{pypi_source} without the name argument is deprecated.
Added the name.
> URL:
https://pypi.python.org/pypi/%{pypi_name}
Two things here:
1. the URL cannot be copied from the spec and pasted to a browser, which is
not nice for a packager who has the spec in front of them and need to go to
the website
2. upstream lists
http://github.com/zopefoundation/zope.hookable as their
website, not PyPI and I belive we should use the same URL as upstream
Corrected both. I had gotten the impression that URLs full of variables were
fine because "more variables" but I did find it annoying trying to then use
those URLs to grab sources.
> Summary: Efficient creation of hookable objects
The second summary could be DRY'ed by using:
Summary: %{summary}
Thanks
The big %if/%else between EPEL and Fedora could use a few empty lines
around
the %else statement to make it clearer it's not part of the %install section.
Fixed so it's more clear.
> -k "not test_pure_python"
This could use an explanation in a comment.
Removed that exclude because those tests actually pass.
> %{buildroot}/%{python3_sitearch}/zope/hookable
This is unusual after %pytet --pyargs. Is it indeed needed? What about plain
zope.hookable here?
plain zope.hookable works. Fixed.
And finally, my favorite:
> %global pypi_name zope.hookable
Is this worth having just for the 3 usages in the spec when everywhere else
we need to use zope-hookable or zope/hookable anyway?
Personally I like the variables so I've opted to leave it here. I've seen them
both ways and even seen the variable in specs slightly simpler than this.
:shrug:
As always, thank you for the feedback!
https://src.fedoraproject.org/rpms/python-zope-hookable/blob/rawhide/f/py...
--
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2117112