Bug 492087 - Review Request: pidgin-latex - A Pidgin plugin that displays LaTeX equations as images in your conversations
Summary: Review Request: pidgin-latex - A Pidgin plugin that displays LaTeX equations ...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-03-25 11:07 UTC by Susi Lehtola
Modified: 2009-04-13 19:47 UTC (History)
3 users (show)

Fixed In Version: 1.3-2.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-13 19:47:42 UTC
lkundrak: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Susi Lehtola 2009-03-25 11:07:14 UTC
Spec URL: http://theory.physics.helsinki.fi/~jzlehtol/rpms/pidgin-latex.spec
SRPM URL: http://theory.physics.helsinki.fi/~jzlehtol/rpms/pidgin-latex-1.3-1.fc10.src.rpm

Description:
Use LaTeX formulas in your pidgin conversations!

The plugin looks for $$latex math$$ patterns, and replaces them by the rendered
latex output by way of latex and imagemagick. The graphics is only displayed
locally, so your conversation partner will have to use the plugin, too 
(With MSN, this constraint might not hold). This plugin is compatible with the 
similar kopete latex plugin, as it uses the $$ as delimiters, too.


rpmlint output clean.

Comment 1 Lubomir Rintel 2009-04-09 05:57:08 UTC
The package is beautiful, nothing to complain for, so I'll made up :)
None of this are blockers, of course.

1.) How about removing the first line from description and replacing the summary with it? An one-line paragraph lines don't look well in descriptions and such short and concise sentence seems to be a good replacement for the long summary line.

2.) Lack of empty line between %prep and %build seems like a sin against the style.

3.) Though technically this does not require pidgin, it is of not much use without it. How about adding pidgin to requires?

4.) I believe that the patch is unneeded, you can accomplish the very same effect by passing LIB_INSTALL_DIR= parameter to make

- Spec file is clean, legible and written using valid American English
- Source tarball matches upstream
- Rpmlint is happy
- Compiler flags used properly
- License is correct, license file included
- No -devel subpackage, no libraries installed
- Provides are sane
- Requires are sane
- Filelist is ok

APPROVED

Comment 2 Susi Lehtola 2009-04-09 08:13:24 UTC
Thanks for the review.

Actually, pidgin IS a requirement since it owns the directory :)
And the patch was not needed.


New Package CVS Request
=======================
Package Name: pidgin-latex
Short Description: Use LaTeX formulas in your pidgin conversations 
Owners: jussilehtola
Branches: F-10 F-11 EL-5
InitialCC:

Comment 3 Kevin Fenzi 2009-04-09 23:11:39 UTC
cvs done.

Comment 4 Fedora Update System 2009-04-10 01:02:53 UTC
pidgin-latex-1.3-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/pidgin-latex-1.3-2.fc10

Comment 5 Fedora Update System 2009-04-13 19:47:37 UTC
pidgin-latex-1.3-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.