[Bug 1279112] Review Request: kiss-fft - Fast Fourier Transform library

[bugzilla at redhat.com]

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

František Dvořák <valtri at civ.zcu.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: kissfft -   |Review Request: kiss-fft -
                   |Fast Fourier Transform      |Fast Fourier Transform
                   |library                     |library



--- Comment #3 from František Dvořák <valtri at civ.zcu.cz> ---
Thank you for the comments and checks.



(In reply to Dmitry Mikhirev from comment #1)
> An informal review.
> 
> * The correct version number should be 1.3.0, not 130 (according to folder
> names at http://sourceforge.net/projects/kissfft/files/kissfft/).

Good catch, it is also in the ChangeLog this way.

> * As the complete library name is "Kiss FFT" and upstream source archive is
> named kiss_fft, probably the more correct name for package should be
> delimited, kiss-fft.

OK. I weren't sure about the name:
* SF project name and header path are "kissfft"
* README is not consistent
* archive is kiss_fft

The "kiss-fft" will be better.

> * Building only static libraries seems reasonable.
> 

Yes, it looks like the kissfft library is supposed to be bundled and compiled
in the project using it, and there is no versioning of the library interface.

> 
> Everything is OK.



(In reply to Antonio Trande from comment #2)
> If you just need to package static libs and header files, I think it's
> better naming this package 'kissfft-static' and providing header files
> separately in 'kissfft-devel'.
> 

With 'kissfft-devel' and 'kissfft-static' one could expect source package
'kissfft'. I would prefer to not changing it (only renamed to 'kiss-fft' as
Dmitry have suggested). It would be also ready for possible transition to the
shared libraries.

But it is good point. I agree I don't have strong arguments here (and it is
probably more about "packaging style").

> - These lines are redundant:
> 
> >%dir %{_includedir}/%{name}/
> >%{_includedir}/%{name}/kfc.h
> >%{_includedir}/%{name}/kiss_fft.h
> >%{_includedir}/%{name}/kiss_fftnd.h
> >%{_includedir}/%{name}/kiss_fftndr.h
> >%{_includedir}/%{name}/kissfft.hh
> >%{_includedir}/%{name}/kiss_fftr.h
> 
> You may use only:
> 
> %{_includedir}/%{name}/
> 

OK, more simple spec file can be better. Updated.

> - Why 'numpy' and 'Python2' among BR packages?

Python is used in the tests in %check, with the mathematical operations from
numpy.



New version:

Spec URL: http://scientific.zcu.cz/fedora/kiss-fft-1.3.0-1/kiss-fft.spec
SRPM URL:
http://scientific.zcu.cz/fedora/kiss-fft-1.3.0-1/kiss-fft-1.3.0-1.fc24.src.rpm

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