Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ocp - Open Cubic Player for MOD/S3M/XM/IT/SID/MIDI music files https://bugzilla.redhat.com/show_bug.cgi?id=452749 ------- Additional Comments From [EMAIL PROTECTED] 2008-06-24 18:16 EST ------- Partial review:: ocp-0.1.15-alsa.patch: - if ((err=snd_pcm_hw_params_any(alsa_pcm, hwparams))) + err=snd_pcm_hw_params_any(alsa_pcm, hwparams); + if (err < 0) This can be done in one line: + if ((err=snd_pcm_hw_params_any(alsa_pcm, hwparams)) < 0) Have the patches been sent upstream? ocp.spec: Release: 1.2%{dist} Please make it just 1%{dist}. We'll start the review at release 1 and increase that number with each revision until the package is approved. BuildRequires: libogg-devel Redundant, required by libvorbis-devel # ocp contains non-64-bit clean i386 assembly ExclusiveArch: i386 Not necessary. It builds and works (mostly) on x86_64 (tested!). I've talked to one of the developers and he said he's working on fixing it on ppc. He also said it works on sparc. Also, please put all BuildRequires in separate lines and sort them alphabetically. It makes them easier to manage and makes cvs diffs smaller and more readable. There's no need to disable libid3tag support, it is present in Fedora (needs BR: libid3tag-devel). There's no reason to disable OSS support, either (no additional BRs or Requires:) /etc/X11/wmconfig/opencubicplayer What's that doing here? I thought these were long obsolete. Additionally, no package owns /etc/X11/wmconfig anymore, so you'll leave unowned /etc/X11/wmconfig directory after ocp is uninstalled. # mv docs to docdir mkdir -p %{buildroot}%{_docdir}/%{name}-%{version} mv %{buildroot}%{_datadir}/%{name}-%{version}/{AUTHORS,BUGS,COPYING,CREDITS,KEYBOARD_REMAPS,SUID,TODO} \ %{buildroot}%{_docdir}/%{name}-%{version} That's not how it's done. Just put those files in %doc. mv %{buildroot}%{_datadir}/applications/opencubicplayer.desktop \ %{buildroot}%{_datadir}/applications/fedora-ocp.desktop Not necessary. Just use --delete-original in desktop-file-install call. -- 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, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
