[Bug 667226] Review Request: tepsonic - A simple, fast and lightweight Qt audio player

[bugzilla at redhat.com]

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

--- Comment #8 from Volker Fröhlich <volker27 at gmx.at> 2011-01-15 18:37:04 EST ---
No, Gettext is only necessary if the project uses .po/.mo. The Phonon Requires
is also not necessary, as RPM finds that on it's own.

I erroneously checked the original desktop file the first time. You can delete
the sed call, because --add-category solves the syntax error. But "application"
is regarded as invalid, so:

desktop-file-install --dir=%{buildroot}/%{_datadir}/applications
--remove-category="Application" --add-category="AudioVideo" %{name}.desktop

You're using custom permissions for installing the executable. After
compilation its 755, after installing 555. I removed the custom settings with
this rough sed:

sed -i '/PERMISSIONS/d' player/CMakeLists.txt

Afterwards, the file section looks like that:

%files -f %{name}.lang -f lastfmscrobbler.lang
%defattr(-,root,root,-)
%doc LICENSE README
%{_bindir}/*
%{_libdir}/%{name}/*.so.*
%{_datadir}/applications/*
%{_datadir}/icons/*


Some cosmetic improvements:

You can also simplify to %cmake -DCMAKE_INSTALL_LIBRARY_DIR=%{_libdir}/%{name}
.

You can use something like "%patch0 -p1 -b .qxt", so you can have a look at the
patched sources, if necessary, and get a kind of comment as well, as a side
effect.

If you don't need the .so link in general, you may consider the use of
NAMELINK_SKIP in CMakeLists.txt. For packaging, deleting it is also fine. If
you stay with rm: Your comment there says, it would delete all NON-developer
libs, which is wrong, I think.


That should be it. I'll do the formal review after you submitted your changes.

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