[Bug 2531] Review request: moc - Music on Console - Console audio player for Linux/UNIX

This is an informal review since I can't sponsor you. ------------------------ Key: [+] - OK [-] - FIX/Needs work. [x] - Not applicable ------------------------- MUST ITEMS =========== It is correct, but you can use this URL as Source URL: Source0: ftp://ftp.daper.net/pub/soft/moc/unstable/%{name}-%{version}-beta1.tar.z2

[-] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines; inclusion of those as BuildRequires is optional. Apply common sense. -- You need to add libtool as BuildRequires. I can't build this package in mock without libtool.

[-] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. -- Libtool archives (*.la files) should not be included. The may need to removed before packaging. See [3] (second paragraph) for more information. You can find an examples here: http://pkgs.fedoraproject.org/cgit/libogg.git/tree/libogg.spec

SHOULD ITEMS ============= [-] SHOULD: The reviewer should test that the package builds in mock. -- You must add libtool as BuildRequires. See MUST ITEMS above.

ADITIONAL ITEMS =============== * Add NEWS in %doc; * line 45: %setup -q -n moc-2.5.0-beta1 We can use macros here. Use %{name} instead of moc and %{version} instead of 2.5.0: %setup -q -n %{name}-%{version}-beta1 * I think it would be better if we add build dependencies as pkgconfig modules, if possible. For example, the package ncurses-devel provides pkgconfig(ncurses) (run rpm -q --provides ncurses-devel | grep pkgconfig). So, we can use BuildRequires: pkgconfig(ncurses) instead of BuildRequires: ncurses-devel But it is just an idea. You can find an example here: http://pkgs.fedoraproject.org/cgit/gpaste.git/tree/gpaste.spec * I think it is better if we add some build options explicitly: %configure --disable-static --without-rcc \ <indentation here>--with-oss --with-alsa --with-jack --with-aac --with-mp3 \ <indentation here>--with-musepack --with-vorbis --with-flac --with-wavpack \ <indentation here>--with-sndfile --with-modplug --with-ffmpeg --with-speex \ <indentation here>--with-samplerate --with-curl

* Tell upstream that FSF for files decoder_plugins/mp3/xing.c and decoder_plugins/mp3/xing.h (see the first must item above) is outdated. Or can provide a patch, if you want.

* Don't forget to update your changelog. That's all :)