Bug 492087 - Review Request: pidgin-latex - A Pidgin plugin that displays LaTeX equations as images in your conversations
Review Request: pidgin-latex - A Pidgin plugin that displays LaTeX equations ...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-25 07:07 EDT by Susi Lehtola
Modified: 2009-04-13 15:47 EDT (History)
3 users (show)

See Also:
Fixed In Version: 1.3-2.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-13 15:47:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Susi Lehtola 2009-03-25 07:07:14 EDT
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 01:57:08 EDT
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 04:13:24 EDT
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 19:11:39 EDT
cvs done.
Comment 4 Fedora Update System 2009-04-09 21:02:53 EDT
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 15:47:37 EDT
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.