https://bugzilla.redhat.com/show_bug.cgi?id=1298665
--- Comment #8 from al...@mellanox.com ---
(In reply to Michal Schmidt from comment #7)
> > ExcludeArch: %{arm} %{ix86} s390x ppc64 ppc64le
>
> Every use of ExcludeArch should have a comment explaining the reason.
I will fix it.
> > %description
> > libvma is a LD_PRELOAD-able library that boosts performance
> > of TCP and UDP traffic
>
> Missing a dot to end the sentence.
I will fix it.
> > Part of Mellanox's enhanced services
>
> Does it mean libvma works only with Mellanox hardware?
> If yes, please rephrase it in a clearer way.
> If it does not mean that, then does it mean anything at all?
libvma can be supported by RDMA capable devices that support
IBV_QPT_RAW_PACKET QPs.
I will rephrase the description.
> > Allows application written over standard socket API
> > to run over Infiniband/Ethernet from user space with full network stack
> > bypass
> > and get better throughput, latency and packets/sec rate.
>
> It allows applications [...]
I will fix it.
> > %pre
> > if [ `grep "^[^#]" /etc/security/limits.conf /etc/security/limits.d/* 2>
> > /dev/null |grep memlock|grep unlimited | wc -l` -le 0 ]; then
> Looks like it could be simplified to:
> if ! grep -qs '^[^#].*memlock.*unlimited' /etc/security/limits.conf
> /etc/security/limits.d/*; then
>
> Anyway, do you really need to emit the message?
> It is unusual for a scriptlet in Fedora to emit any output whatsoever.
We print this message in order to notify about system configuration change
(memlock limit). I agree that this is not crucial and can be removed.
> > %files
> > %{_libdir}/%{name}*.so.*
> > %{_libdir}/%{name}.so
>
> You may want to add a comment here saying you need libvma.so in the main
> package
> (as opposed to -devel) so that 'LD_PRELOAD=libvma.so <command>' works.
Good idea, I will add this comment.
> > %doc README.txt journal.txt VMA_VERSION
>
> I see your upstream tarball contains a COPYING file, that you're not
> including in the package.
> Ideally it would contain the full text of the GPLv2
> (Would you please make that change upstream?
> Currently it has only a copy of the header as it appears in the source
> files.)
I am checking if we can make this change.
> and be included in the package using the %license tag (not %doc).
I will add the COPYING file to the package under the %license tag.
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
