https://bugzilla.redhat.com/show_bug.cgi?id=1110386 František Dvořák <valtri@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |valtri@xxxxxxxxxx Assignee|nobody@xxxxxxxxxxxxxxxxx |valtri@xxxxxxxxxx Flags| |fedora-review? --- Comment #2 from František Dvořák <valtri@xxxxxxxxxx> --- Taking the review... 1) License: the license is rather LGPLv2; but there is one (only) GPL-ed file - /usr/share/codec2/scripts/menu.sh Maybe it is not needed to instal it, or alternativelly you can use GPLv2 license for the -example subpackage? In eihter way, you can ask upstream about licensing: they are intended to use LPGL for codec2 project and maybe they would rather relicense the menu.sh file under the same licesne? 2) Comment in the source section: * It seems SourceForge changed repository URLs, in this case to https://svn.code.sf.net/p/freetel/code/ * Is the step of taking the codec2-dev proper? There are some differences from the tarball in .src.rpm and codec2-dev (missing codec2/voicing, addidional dependency on speex, no pre-built binaries in win32) 3) Pre-built binaries in win32 should be removed in %prep [http://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-built_binaries_or_libraries] 4) The checkout information in release version should have the form of YYYDDMMsvnREV [http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages] For example (release field): 1.20140616svn1657 5) pkgconfig is not needed in Requires, it is generated automatically in both Fedora and EPEL 6 (as /usr/bin/pkg-config) 6) Is the build inside build_linux needed? (I guess you use it because it is mentioned in READMEs?) There is no formal or technical problem with that, only the build steps could be slightly simpler without it. :-) 7) It is recommended to track major version of the libraries in %files, like: %{_libdir}/libcodec2.so.0 %{_libdir}/libcodec2.so.0.* 8) ChangeLog, NEW, AUTHORS are empty files, they're not needed to install 9) README.cmake is only about build instrutions and it is not needed 10) codec2-examples should have dependency on "%{name} = %{version}-%{release}", not the -devel. Was there any reason for that? 11) rpmlint: E: non-executable-script /usr/share/codec2/script/*.sh 12) rpmlint: W: mixed-use-of-spaces-and-tabs (spaces: line 20, tab: line 7) 13) Description should end with dot. :-) Enhancements: 14) Is possible to use something in %check? There is a testsuite, but if I understand corretly, it is not intended for automatic testing. (it is more for codec developers?) It could be commented in the .spec file that the testsuite exists, but it can't be used. And maybe it could be used something like this instead?: src/c2enc 3200 raw/mmt1.raw test.c2 src/c2dec 3200 test.c2 test.raw test -s test.c2 -a -s test.raw It is not real codec testing, it will just check it doesn't crash (on the all platforms). 15) man-pages: there is recommended (but not required) man-page for each binary in /usr/bin [http://fedoraproject.org/wiki/Packaging:Guidelines#Man_pages] -- 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