Bug 479750 (libcelt)

Summary: Review Request: libcelt - low-latency compressed audio format for voice and music
Product: [Fedora] Fedora Reporter: Monty <cmontgom>
Component: Package ReviewAssignee: Ray Strode [halfline] <rstrode>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, kem, lemenkov, notting, pbrobinson, rstrode
Target Milestone: ---Flags: rstrode: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-01-13 17:54:59 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 478941    
Bug Blocks:    

Description Monty 2009-01-12 21:14:38 UTC
Spec URL: http://people.xiph.org/~xiphmont/CELT/libcelt.spec
SRPM URL: http://people.xiph.org/~xiphmont/CELT/libcelt-0.5.1-0.fc10.src.rpm
Description: CELT is a fully open, non-proprietary, patent- and royalty-free,
low-latency compressed audio format for voice and music.  The libcelt package contains runtime libraries for use in programs that support Ogg CELT.

Note that the package name is consistent with the other packaged Xiph codec libraries; the upstream is named 'CELT' [as opposed to libcelt] mostly because the developer who handled the initial release engineering was partially unaware of the naming scheme across the Xiph repository.

Comment 1 Ray Strode [halfline] 2009-01-12 22:11:14 UTC
The only real issue i see is that you need to

Requires: pkg-config

since you ship .pc files

Other than that, rpmlint gave a few warnings that you may want to fix up:

1) you run make check in %build but normally you'd run that in %check
2) you mix tabs and spaces
3) the tarball in the srpm is 0600 instead of 0644 or whatever
4) You're changelog entry doesn't have a version at the end of it.

Normally you'd do something like:

* Mon Dec 22 2008 Monty Montgomery <monty> 0.5.1-0
- Initial spec file

Last thing I'd bring up is you have an
Epoch: 0
and
Requires: libcelt = %{epoch}:%{version}-%{release}

but often epochs aren't explicitly given unless they're non-zero.

Anyway, the only change you really need afaict is the Requires for pkg-config

Comment 2 Monty 2009-01-12 22:23:52 UTC
>The only real issue i see is that you need to
>Requires: pkg-config
>since you ship .pc files

Fixed

>1) you run make check in %build but normally you'd run that in %check

Changed

>2) you mix tabs and spaces

Emacs is doing that mainly because the .emacs file config to make it stop seems to change every minor release and I'm sick of fixing it every time I update.

>3) the tarball in the srpm is 0600 instead of 0644 or whatever

Changed

4) You're changelog entry doesn't have a version at the end of it.

Fixed

I've refreshed the files at http://people.xiph.org/~xiphmont/CELT with the above changes.

Comment 3 Ray Strode [halfline] 2009-01-12 22:27:35 UTC
looks good.

Comment 4 Monty 2009-01-12 22:32:39 UTC
New Package CVS Request
=======================
Package Name: libcelt
Short Description: The CELT Low-Latency Audio Compression Codec
Owners: xiphmont
Branches: F-10 
InitialCC:

Comment 5 Peter Lemenkov 2009-01-13 09:58:46 UTC
Please look at the package review queue befora submitting review

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

Comment 6 Peter Robinson 2009-01-13 10:41:15 UTC
This is the same as my package review. And the package is not labelled according the the fedora policy.

Comment 7 Ray Strode [halfline] 2009-01-13 14:47:44 UTC
Hi Peter,

Monty explained why the name is "libcelt" instead of "celt" in his initial comment.

Do you guys mind being comaintainers with each other?

Comment 8 Monty 2009-01-13 17:44:21 UTC
I'm happy to do it any which way.  I'm part of the upstream team, so it makes sense especially as the codebase is brand new.

Comment 9 Ray Strode [halfline] 2009-01-13 17:54:59 UTC
Alright, let's close this report out since the other one was filed first.

*** This bug has been marked as a duplicate of bug 478941 ***

Comment 10 Peter Robinson 2009-01-14 03:46:03 UTC
I'm happy with co-maintainer, I initially packaged it so I could add support to ekiga. Will update the other bug report.