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



--- Comment #23 from Andrey Maslennikov <[email protected]> ---
> Perhaps the purpose of your personal fork (amaslenn/ucx) is to make the 
> package pass this review and then you plan to merge the changes into the 
> openucx/ucx repository and do a real (and no longer changing) 1.2.2 release?
> Do you then intend to change the Source tag in the spec file to point to 
> https://github.com/openucx/ucx?
That is exactly my purpose.


>> %{_datadir}/doc/ucx
> Did you intentionally avoid using %{_pkgdocdir}? Is it because older distros 
> do not define the macro? You could do what some other Fedora packages did for 
> compatibility with EPEL 6:
> %{!?_pkgdocdir: %global _pkgdocdir %{_docdir}/%{name}-%{version}}
Yes, I expect issue with old distros we support. Should I add this WA and use
%{_pkgdocdir} instead?


>> %doc %{_datadir}/doc/ucx/examples
> I believe it is unnecessary to use the explicit "%doc" marking. RPM 
> automatically marks files under _pkgdocdir as documentation.
Will move %{_datadir}/doc/ucx/examples out of %doc tag.


> I am not sure how safe it is to mix the usage of both %doc with relative 
> paths (for README, etc.) and _docdir / _pkgdocdir. The guidelines forbid it 
> if I'm reading them correctly:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation
> I see no obvious issue in the built packages, but maybe it could cause 
> trouble with other versions of RPM. Better avoid the mixed usage by 
> installing the files README, AUTHORS, NEWS into _pkgdocdir in the %install 
> step instead of relying on %doc with relative paths.
Considering the previous paragraph, no issue here, right?


>> # UCX ships both static and dynamic libs to support different use-cases
> I still don't get what the usecase is. Is it for performance reasons?
Yes.


> I see the sources include some unit tests. Have you considered running them 
> in the %check step of the rpm build?
Tests are HW dependent, so we won't use them in rpm build. Running tests is a
separate stage of our CI, and release package won't be approved without all
tests passing.

-- 
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 -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to