[Bug 539837] Review Request: mppenc - Musepack audio compressor

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #3 from Michael Schwendt <mschwendt at gmail.com>  2009-11-21 09:26:50 EDT ---
Thanks for the quick review!

As of an RPM update in Fedora 10 and later,

* %buildroot is defined by default,
* BuildRoot definition in the spec file is ignored,
* %buildroot tree is removed automatically at beginning of %install,
* a default %clean section is provided by rpmbuild.

This package actually is my first public one to omit all these items as they
are no longer necessary.

See "rpmbuild --clean --rebuild mppenc-1.16-0.1.fc11.src.rpm"

> Is there a good reason to go against suggestions from rpmdev-newspec ?

Personal taste. The order in rpmdev-newspec was completely arbitrary and based
on personal taste, too.

> ossaudio

mpp.h says: 
| on some systems you also must link the libossaudio library,
| so maybe you also must edit the Makefile

However, with Fedora there is no dependency on anything like that. No extra
header inclusion either. The code path, which evaluates the USE_OSS_AUDIO
preprocessor variable, doesn't do anything special. The code reuses a header
and definitions from the mpc decoder mppdec.

> 5. Install also mentions use of:
> 'cmake -DCMAKE_INSTALL_PREFIX:=/myPath'
> Seems it installs into a fedora happy location anyway.

See "rpm --eval %cmake".

> 6. patch cmake. Is this to ignore the default build params and
> ensure Fedora's get used. It might be useful to explain that
> above the patch definition,

Good suggestion, as I normally do that unless a patch is really obvious.

> 7. license:

A mail to upstream is on its way.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.