https://bugzilla.redhat.com/show_bug.cgi?id=1279112 František Dvořák <valtri@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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 _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review