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

František Dvořák <val...@civ.zcu.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |val...@civ.zcu.cz
           Assignee|nob...@fedoraproject.org    |val...@civ.zcu.cz
              Flags|                            |fedora-review?



--- Comment #2 from František Dvořák <val...@civ.zcu.cz> ---
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to