[Bug 1077030] Review Request: python-semantic-version - library implementing SemVer

bugzilla at redhat.com bugzilla at redhat.com
Fri Jul 3 08:32:30 UTC 2015


https://bugzilla.redhat.com/show_bug.cgi?id=1077030



--- Comment #12 from Miroslav Suchý <msuchy at redhat.com> ---
> Source0: https://github.com/rbarrois/python-semanticversion/archive/v2.4.2.tar.gz

Unless upstream change the format each release, you should rather use:

Source0:
https://github.com/rbarrois/python-semanticversion/archive/v%{version}.tar.gz

as it will keep the spec' diff minimal on each release.


> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

BuildRoot is not needed since F10
  https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
Unless you want to build this for EL5 you should remove it.

Can you move %check after %install? This will easy reading to our pure humans.
Because rpmbuild first execute %prep, then %build, followed by %install and
only then %check. So if you sort the section in your spec this way, is more
readable.


> export LC_ALL=en_US.UTF-8
> %{__python3} setup.py build

This should be rather rewritten as:
LC_ALL=en_US.UTF-8 %{__python3} setup.py build

so the LC is set just for that one command.
Similarly in %install section.


> pushd docs && make html
> popd && mv docs/_build/html htmldocs

I see no reasons why to use &&. All scriptlets are execute with "-e" therefore
if any command will fail, build will  immediately stopped.
In fact if docs directory will be missing, the first line will just mask it.
And second line will either fail or popd into unexpected directory.

> %defattr(-,root,root,-)
Again - not needed unless you aim EL5.

> %{!?_licensedir:%global license %%doc}
no need to define it twice. And I woule personally move it to top of the file,
where other macros are defined.

I would consider putting htmldocds in separate -doc subpackage. It is quite
big, especially because it bundle jquery. But I will accept if you hesitate to
create it (with some reasoning).

-- 
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


More information about the package-review mailing list