https://bugzilla.redhat.com/show_bug.cgi?id=1474033 --- Comment #18 from Andrey Maslennikov <andreyma@xxxxxxxxxxxx> --- >> %{!?configure_options: %global configure_options %{nil}} > How do you intend to use this? It will always be nil when the package is built in the Fedora build system. >> Group: System Environment/Libraries >> [...] >> BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) > > "Group:" and "BuildRoot:" SHOULD NOT be used according to > http://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections > > "SHOULD NOT" is not as strong as "MUST NOT", but sharing the same spec file between Fedora and SLES does >not seem like a worthwhile goal in the long run. IMO it's not a valid reason for having these tags. Supporting several specs is more complicated especially taking into account that the diff will be in 1-2 lines only. So I'd like to keep it this way, the line is required. It was proved by some fails in our build env :) >> BuildRequires: automake autoconf libtool > It seems you're only running ./configure in the %build step, not regenerating it. Does the build really require autotools? Will check. >> the following shared memory mechanisms: posix. sysv, cma, knem, xpmem. > Typo. This period should be a comma -----------^ Will fix, thanks. >> %package static > The packaging guidelines say: "In general, packagers are strongly encouraged not to ship static libs unless a compelling reason exists." > So a comment stating a reason for including the static libs would be nice. Will add a comment. In fact, static libs are simply required to develop with UCX. >> make %{?_smp_mflags} V=1 > Just a minor suggestion: We have a macro called "make_build", defined as: > %{__make} -O %{?_smp_mflags} > It would be nice to use that macro: > %make_build V=1 Will check if it works for as and switch if yes. >> make DESTDIR=%{buildroot} install > There is a macro for this as well: %make_install Will update, thank you. >> %{!?_licensedir:%global license %%doc} > Is this to make the spec file work in older distros? It is. Need a comment? > Other: > "[...] you must list a BuildRequires against gcc, gcc-c++ or clang" > http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires Will add. > Please insert empty lines between %changelog entries. Will add. -- 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
