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

Richard Shaw <hobbes1...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED



--- Comment #3 from Richard Shaw <hobbes1...@gmail.com> ---
(In reply to František Dvořák from comment #2)
> 1) License: the license is rather LGPLv2; but there is one (only) GPL-ed
> file - /usr/share/codec2/scripts/menu.sh

That file isn't required so it could be removed.


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

I'll ask but I may just change it. While I am not specifically a programmer, I
am a contributor and have commit access to upstream as I wrote and now maintain
their cmake build system.


> 2) Comment in the source section:
> 
>   * It seems SourceForge changed repository URLs, in this case to
> https://svn.code.sf.net/p/freetel/code/

Yes, forgot that happened after I submitted the package.


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

The orginal codec2 branch was stagnant and codec2-dev was the "working" branch.
It's a very small upstream team and not terribly disciplined :) I have since
moved a known good copy of codec2 while codec2-dev continues to be developed. I
just need to update the spec file.


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

Will do.


> 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

The is more or less 0.3 official (as it gets) we don't yet do formal release
archives. The irony of the guidelines is that svn1657 tells you exactly what
svn revision it is, but it's not required, but the date of the checkout is
required. I disagree with the guidelines here but the rules are the rules so
I'll fix it.


> 5) pkgconfig is not needed in Requires, it is generated automatically in
> both Fedora and EPEL 6 (as /usr/bin/pkg-config)

Ahh. Didn't know that.


> 6) Is the build inside build_linux needed? (I guess you use it because it is
> mentioned in READMEs?)

Both cmake and I prefer out of source builds. It's in the readme because I
wrote it. :)


> 7) It is recommended to track major version of the libraries in %files, like:
> 
> %{_libdir}/libcodec2.so.0
> %{_libdir}/libcodec2.so.0.*

I can change it if you feel strongly about it but I actually manage the
soversion so I'm not too worried.


> 8) ChangeLog, NEW, AUTHORS are empty files, they're not needed to install

I usually include them even if they are empty because they may not always be
empty and who actually checks for that during the update process?


> 9) README.cmake is only about build instrutions and it is not needed

I should have caught that since I wrote it :)


> 10) codec2-examples should have dependency on "%{name} =
> %{version}-%{release}", not the -devel. Was there any reason for that?

Maybe I should rename the package to codec2-devel-examples? It's not needed for
the main package at all.


> 11) rpmlint: E: non-executable-script /usr/share/codec2/script/*.sh

I'll fix these in svn rather than work around it in the package.


> 12) rpmlint: W: mixed-use-of-spaces-and-tabs (spaces: line 20, tab: line 7)

How did that sneak in there? I always use rpmdev-newspec to generate specfile
templates...


> 13) Description should end with dot. :-)

Will fix.


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

It's not designed to be automated yet. My plan is to add automatic testing via
ctest but the current developer is more interested in codec2-dev than codec2 so
I'm not sure if it will happen anytime soon.

In fact, I'm moving the binaries to the devel package as they are not terribly
useful except for development (and testing) purposes. 


> 15) man-pages: there is recommended (but not required) man-page for each
> binary in /usr/bin

I would like them to have one as well but I doubt I can get upstream to write
them.

I'll try to get a new SRPM and SPEC posted today if I have time.

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