[Bug 453422] Review Request: songbird - Mozilla based multimedia 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=453422





--- Comment #29 from Nicolas Chauvet (kwizart) <kwizart at gmail.com>  2009-03-02 08:53:56 EDT ---
Well, since there is a 1.1 beta2 release two days ago, I wonder if it wouldn't
be suitable to concentrate a this 1.1 pre-release while targeting Fedora-11.
That would be easier to have fix merged if any.

Then you should be able to backport changes to 1.0 eventually for F9/F10.

Just a quick notes,(don't update the spec just for this at this time):
Uses Source0:  instead of Source:

While using desktop-file-install, please don't use "fedora" as vendor leaving
it empty ("") for now.
See http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
(that's a rather new guideline whereas it was common practice earlier).

Use cp -p to prevent timestamps changes while copying various files/directory. 

Don't hardcode the parallel build macro:
make -f songbird.mk MOZ_MAKE_FLAGS=-j4
make -f songbird.mk MOZ_MAKE_FLAGS=%{?_smp_mflags}
It should allow our builder to build at the maximum speed, If the parallel
build aren't safe, you should have a comment. same if the jobserver isn't
supported from a given makefile.

%post and %postun. Are you really sure you need to run ldconfig at %post and
%postun (thunderbird doesn't do that at least). 
Having update-desktop-database there is good since you have mime type
Can you see this http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
Specially, you should try to use GTK+ icon cache (with various icons size if
possible) and avoid using the pixmap directory.
Please use "|| :" to avoid scriplet failure if something went wrong for any
reason.

Do you really need "BuildRequires: zip" ? I guess this is a compression
utility. You shouldn't need it to decompress anything, aren't you ?
Unless it is used within the buildsystem... I haven't checked yet.


At this step, it was still a preliminary review at the "looking at the spec"
level.

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