[Bug 185951] Review Request: amsn : msn messenger clone

[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: amsn : msn messenger clone


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





------- Additional Comments From wart at kobold.org  2006-06-19 17:08 EST -------
(In reply to comment #27)
> (In reply to comment #25)
> 
> > * Missing "Requires: mozilla".  I would recommend, however, changing the
> >   default browser command from 'mozilla $url' to 'htmlview $url' and
> >   adding "Requires: htmlview".  This will launch the url in the
> >   user's preferred web browser.
> I changed mozilla to htmlview but I am not sure about requiring it. aMSN runs
> fine without it, only when you try to open an url it tells you you should check
> your preferences. People not having htmlview have chosen for that i guess, since
> it is installed by default.

I still think you should add "Requires: htmlview, sox".  If the application  
uses it, it should be in the Requires: list.  Even though htmlview is
installed by default, it's best to make sure that the package has it
available.  Fortunately, amsn won't crash if it's not there, it will just
print out a warning that the user needs to modify their configuration.
I don't think the additional Requires: will hurt anything.

> > * Missing "Requires: sox" for /usr/bin/play for playing sounds.
> Same as htmlview, aMSN runs fine without sound.

See above.

> > QUESTIONS
> > =========
> > * Official upstream sources come from sourceforge.net, but this rc release
> >   comes from elsewhere.  Since I know you're one of the upstream developers,
> >   I'm not too concerned about this, but it would be better if Source0:
> >   pointed to upstream's official download page.
> Sorry, I meant to write the reason in my previous comment but forgot. At the
> time I had trouble uploading the file to sf, but in the meantime i have done so.
> I changed Source0 accordingly. Just be aware that the release is still hidden so
> you won't find the file on the project website yet.

Are there plans to make the RC1 release public?  It'd be really nice to have a
working Source0: link in the spec file that points to SF.  Right now I can't get
to it using curl, wget, or a browser.

> > * You might consider submitting cximage as a separate package instead of
> >   including it in amsn.  Since Fedora doesn't include cximage already,
> >   this won't block the review.
> The CxImage supplied with amsn is much different from upstream, and i think not
> all patches were accepted so it will stay that way but i am not sure. (I will
> check if this is really true)

Ok.

> > * Is there a special reason why the amsn commands (amsn, amsn-remote,
> >   amsn-remote-CLI) are located in %{_datadir}/amsn and only linked to
> >   %{_bindir}?  Why can't they just be installed directly into %{_bindir}?
> Yes, the file perform some magic to see where they are located, and set some
> variables accordingly to be able to find plugins etc.

Ok.  That makes sense.  You might suggest to upstream :) that they validate
$program_dir before trying to resolve symlinks.  This would allow you to use sed
in the spec file to set program_dir to %{_datadir}/%{name} and bypass the
symlink dereferencing in the program.  As a result, the program should still
work as it does now, but it would also work if amsn weren't installed in the
data directory.

A few other items:

MUSTFIX
=======
* Remove the extra " from the comment in the .desktop file
* It turns out that /usr/share/amsn/README is needed at runtime for the
  About box.  You can go ahead and remove it from the list of doc files
  that are deleted during %install.  Do you know of any other doc files
  that are used at runtime?

SHOULD
======
* The Help -> Contents menu item doesn't seem very useful, since it
  describes how to install and start amsn, which the user must have
  already done if they can activate the menu.  I'd suggest to upstream that
  they either remove this menu item, or replace the Help ->Contents text with
  something more useful.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.