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=799651 Rex Dieter <rdieter@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@xxxxxxxxxxxxxxxxx |rdieter@xxxxxxxxxxxx QAContact|rdieter@xxxxxxxxxxxx |extras-qa@xxxxxxxxxxxxxxxxx Alias| |smb4k Flag| |needinfo?(sergio@xxxxxxxxxx | |) --- Comment #10 from Rex Dieter <rdieter@xxxxxxxxxxxx> 2012-03-27 15:59:55 EDT --- OK, first pass review: naming: ok license: ok sources: ok $ md5sum *.bz2 c4b515e482ef7a7a834a3b660e1ee4d0 smb4k-1.0.1.tar.bz2 macros: ok (in general, some other suggestions below) scriptlets: NOT ok 1. MUST: fix scriptlets for icons, mimetypes, see: https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database 2. MUST: change subpkg dependencies from Requires: %{name} = %{version}-%{release} to Requires: %{name}%{?_isa} = %{version}-%{release} See: https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package 3. SHOULD: I'd recommend changing Requires: %{_bindir}/kdesu to Requires: kdebase-runtime%{?_kde4_version: >= %{_kde4_version}} which will capture a minimal (versioned) kde dependency too 4. MUST change BuildRequires: cmake >= 2.6.0 BuildRequires: kdelibs-devel >= 4.4.0 to BuildRequires: kdelibs4-devel the latter is safer (esp say when/if kdelibs-devel upgrades to a potentially incomatible version 5), and it already pulls in cmake for you. 5. SHOULD omit # Make symlink relative snippet, it's not required anymore 6. MUST document need for the --add-category items in desktop-file-install call 7 SHOULD: in %files, change /usr/libexec/kde4/mounthelper to %{_kde4_libexecdir}/mounthelper 8. MUST drop -devel subpkg. there's only a lib symlink and no exported API/headers. Easier to just delete the symlink (or use %exclude), whatever works for you. OK, I think that's all I've got, if you can address all these items, should be close to approval. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review