[Bug 370561] Review Request: bmpx - Beep Media Player eXperimental

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

Summary: Review Request: bmpx - Beep Media Player eXperimental


https://bugzilla.redhat.com/show_bug.cgi?id=370561





------- Additional Comments From akahl at iconmobile.com  2007-11-18 18:29 EST -------
(In reply to comment #2)
> Well,
> 
> ! License is actually strict GPLv2.
Yes - did I miss anything here?

> * First triggerscripts
>   - Personally, I don't like triggerscripts.
>     However, even if we ignore what I feel, current triggerscripts
>     contains some problems.
[...]
I've considered this a lot after reading your remarks about triggerscripts and
think you are right, if there's any chance to get around triggerscripts one is
usually right to choose that way, they're easily likely to break things and not
easy to grasp in the first place.
 
>      I think the proper way is to rebuild bmpx every time firefox
>      is upgraded, or to separate firefox extension related files
>      from bmpx (and submit a new review request for bmpx-mozextension,
>      for example) if you don't want to rebuild whole bmpx.
The updated spec above creates such an extension, I've nailed the Firefox
compatible version to 2.0.0.9 however I think on the long run we'd be better off
to adapt the current Firefox plugin handling in a version independent directory
for extensions, i.e. we have
/usr/lib[64]/mozilla/plugins for plugins and should use
/usr/lib[64]/mozilla/extensions for extensions.

Do you think I should file a bug report for this issue?


> - For generic issues:
> * koji build
>   - -2 does not build.
>     http://koji.fedoraproject.org/koji/taskinfo?taskID=236212
>     Note: basically, %_iconsdir is not defined.
[...]
Fixed in -3
 
> * Timestamps
>   - Please use "-p" option when you use "cp" or "install" commands
>   - Also, try to add 'INSTALL="install -p"' option to "make install"
>     if this keeps timestamps on name png file, header files or so
>     (I guess yes).
Fixed in -3

> * Exclude
>   - It seems many .la files are installed under %_libdir/bmpx/plugins/taglib.
>     Check if this is correct.
>   ! Note:
>     I prefer not to use %exclude but really to "remove" unneeded files
>     by the time %install finishes.
Fixed in -3

> * %post scriptlet
> ----------------------------------------------------------
> if [ -x %{_bindir}/gtk-update-icon-cache ]; then
> 	update-desktop-database &>/dev/null || :
> fi
> ----------------------------------------------------------
>   - Perhaps this is some copy/paste mistake.
You are right it is a copy/paste mistake, also fixed in -3

> * Files entry
>   - Are Makefile.* under %_defaultdocdir really needed?
No, my fault, sorry! Fixed in -3

> * -devel package?
>   - Well, why is this -devel package needed?
>     bmpx does not contain any libraries in ldconfig default paths,
>     and -devel package does not contain any symlinks for libraries.
> 
>     As far as I see this package, -devel package is completely unneeded.
My opinion differs here. While the -devel subpackge contains no libraries so
far, all files contained within, i.e. header files and the pkgfile are only
useful for bmpx plugin developers and totally useless for pure end users.
However so far I've been unable to find any clarifying guidelines about -devel
subpackages' contents, if you don't know any neither we should use our common
sense here.

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