[Bug 1829] Review request: deadbeef - A music player with cue sheet support

http://bugzilla.rpmfusion.org/show_bug.cgi?id=1829 Richard <hobbes1069(a)gmail.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069(a)gmail.com --- Comment #21 from Richard <hobbes1069(a)gmail.com&gt; 2011-10-06 20:14:49 --- Some comments on the spec file: 1. BuildRoot:, "rm -rf $RPM_BUILD_ROOT" can be removed from %install, %clean can be removed entirely, and "%defattr(-,root,root)" can be removed from %files unless your building for EL5. 2. In you devel subpackage the Require: should probably be arch dependent: Requires: %{name} = %{version}-%{release} to Requires: %{name}%{?_isa} = %{version}-%{release} 3. Modifying the desktop file in %prep is odd. desktop-file-install can modify desktop files on the fly, so if it's capable of removing the Unity parts then I would use it instead. Just research it's options. If desktop-file-install is not capable of making the needed edits and you still need to resort to sed then at least move it to just above your desktop-file-install command. Also, currently you're not modifying anything in the desktop file by the time you get to desktop-file-install so you could use desktop-file-validate instead. See: http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_u... 4. Ok, you definitely took Michael's comments to heart, but perhaps went a little too far :) I would only add comments in %files for unusual situations. You can drop #doc, # Files, and # Dirs. 5. %{_defaultdocdir} can just be %{_docdir} 6. In the case of the extra doc directory there's no hard rule for this that I know of but it's my opinion that %exclude use in %files should be limited to excluding files from one package so they can be included in a sub-package. For instance, you might run into a situation where it would be difficult or overly verbose to work around files that you really want to go into a -devel or -doc subpackage. Here %exclude could be used to exclude files or directors in the main package so they can be included in -doc or -devel. If you're not going to include files in a package at all then it's better to just remove them in %install like you're already doing for .la files. For the static libraries (.a) I would just remove them if there's nothing that actually needs them. If you need them then they should go in a -static subpackage not in -devel. -- Configure bugmail: http://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.