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

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=2531

--- Comment #13 from Antonio <anto.trande at gmail.com> 2012-10-21 12:01:14 CEST ---
(In reply to comment #12)
> 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

Done but rpmlint provides me an error: 
$ rpmlint moc-2.5.0-0.1.beta1.fc17.src.rpm
moc.src: W: invalid-url Source0:
ftp://ftp.daper.net/pub/soft/moc/unstable/moc-2.5.0-beta1.tar.bz2 <urlopen
error ftp error: (null) Down: 0 Files (0mb)  Up: 0 Files (0mb)  10,000,000:1 
CR: LEECH>


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

Done. I have changed 'BuildRequires:ffmpeg-devel' with 

##Build Requires of ffmpeg-devel 
BuildRequires: pkgconfig(libavcodec)
BuildRequires: pkgconfig(libavdevice)
BuildRequires: pkgconfig(libavfilter)
BuildRequires: pkgconfig(libavformat)
BuildRequires: pkgconfig(libavutil)
BuildRequires: pkgconfig(libpostproc)
BuildRequires: pkgconfig(libswresample)
BuildRequires: pkgconfig(libswscale)

Is it right ?

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

Done.

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

Done.

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

I will send a mail about this issue to the upstream maintainer.
Is it disabling for the request ?

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

Thank you.

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