{Not Official Reviewer}
packaging looks ok.
+ Mockbuild is successfull for i386 FC6 with some warnings 
+ rpmlint on binary rpm is silent
+ dist tag is present
+ Buildroot is correct
+ source URL is correct
+ BR is correct
+ License used is GPL
+ License file COPYING is included
+ desktop file is handled correctly
+ MD5 sum on tarball is matching upstream tarball
245ffd3b4a6f968d8f80d42f6aad079c  twinkle-0.8.1.tar.gz
+ No duplicate files

You can use --delete-original with desktop-install-file
Instead of writing
rm -f ${RPM_BUILD_ROOT}%{_datadir}/applications/twinkle.desktop
desktop-file-install --vendor fedora                            \
       --dir ${RPM_BUILD_ROOT}%{_datadir}/applications         \
        --add-category X-Fedora                                 \
        twinkle.desktop

you can write only in SPEC

desktop-file-install --vendor fedora                            \
        --delete-original --dir ${RPM_BUILD_ROOT}%{_datadir}/applications         \
        --add-category X-Fedora                                 \
        twinkle.desktop



Unable to install on FC6 got
error: Failed dependencies:
        libresolv.so.2(GLIBC_PRIVATE) is needed by twinkle-0.8.1-1.fc6.i386

Thanks for the --delete-original tip. Will look at changing that in the next 
version. 

I noticed the failed dependency eariler, and was digging further. 
After much hair pulling I finally found that the lovely 'abicheck' utility can 
show you what private functions a binary is using, so we see: 

/usr/bin/twinkle: PRIVATE: (libresolv.so.2:GLIBC_PRIVATE) __ns_name_ntop
/usr/bin/twinkle: PRIVATE: (libresolv.so.2:GLIBC_PRIVATE) __ns_get16

Those two will need to get patched/replaced. I will see if upstream can do 
anything, or if I can come up with a quick patch to fix it. (Or indeed, if 
anyone watching this bug has a quick fix. ;) 

(In reply to comment #2)
> Thanks for the --delete-original tip. Will look at changing that in the next 
> version. 
> 
> I noticed the failed dependency eariler, and was digging further. 
> After much hair pulling I finally found that the lovely 'abicheck' utility can 
> show you what private functions a binary is using, so we see: 
> 
> /usr/bin/twinkle: PRIVATE: (libresolv.so.2:GLIBC_PRIVATE) __ns_name_ntop
> /usr/bin/twinkle: PRIVATE: (libresolv.so.2:GLIBC_PRIVATE) __ns_get16
> 
> Those two will need to get patched/replaced. I will see if upstream can do 
> anything, or if I can come up with a quick patch to fix it. (Or indeed, if 
> anyone watching this bug has a quick fix. ;) 

You may be able to fix it by persuading the configure script that the function
isn't available, so that it either doesn't use it or provides a replacement. I
did that for __ns_get16 once:

ac_cv_func___ns_get16=no
export ac_cv_func___ns_get16
./configure ...

ok, new version up: 

* Sun Sep 24 2006 Kevin Fenzi <kevin> - 0.8.1-2
- Add bind-libbind-devel as a BuildRequires and link against it
  to fix private glibc symbols.
- Add alsa-lib-devel and kdelibs-devel BuildRequires

Linking against libbind instead of libresolv seems to solve all the private 
symbol issues. 

adding alsa-lib-devel adds ALSA support. 
adding kdelibs-devel to add KDE support. 

Spec URL: http://www.scrye.com/~kevin/extras/twinkle/twinkle.spec
SRPM URL: http://www.scrye.com/~kevin/extras/twinkle/twinkle-0.8.1-2.fc6.src.rpm

Parag: Strictly speaking, you're a vetted contributor, and Kevin certainly is,
so you *can* do official reviews.  If you're afraid of screwing something up, so
be it; I'm continually expecting to make glaring mistakes and get flamed for it,
but that won't stop me from reviewing. :-)

Since I don't do enough reviews (and readily acknowledge this), I think I'll
take this one on.  Looks like something that would certainly benefit Fedora.

Starting the review...well, as soon as my devel branch rsync finishes (I *swear*
I just synched it).

Using my own review checklist:
http://beer.tclug.org/fedora-extras/review-checklist-1.1.txt

1. No rpmlint output from the main or source RPM.  However, from the
debuginfo, I got:
W: twinkle-debuginfo hidden-file-or-dir /usr/src/debug/twinkle-0.8.1/src/gui/.moc
W: twinkle-debuginfo hidden-file-or-dir /usr/src/debug/twinkle-0.8.1/src/gui/.moc
W: twinkle-debuginfo hidden-file-or-dir /usr/src/debug/twinkle-0.8.1/src/gui/.ui
W: twinkle-debuginfo hidden-file-or-dir /usr/src/debug/twinkle-0.8.1/src/gui/.ui

I have no idea how important that is.

2. Package seems to adhere to the Package Naming Guidelines.
3. Spec is %{name}.spec
4. Package appears to meet the Packaging Guidelines.
5. Package is GPL...
6. ...and the spec agrees.
7. COPYING included in %doc
8. Spec is in American English.
9. Spec is legible.
10. Tarball MD5 matches upstream (245ffd3b4a6f968d8f80d42f6aad079c).
11. Packages build on FC5/i386, FC5/ppc, and FC6/i386.
12. n/a, unless it fails to build on x86_64.
13. Package built in Plague, so if there are any missing BRs, they're non-fatal
(and might silently corrupt this package!)
14. Package makes no effort to handle locales.  Oh well.
15. n/a, no shared libraries.
16. Package is not designed to be relocatable.  Are any?
17. Package owns all directories it creates (aside from those owned by filesystem).
18. No duplicate files.
19. Permissions look sane, and there's a %defattr entry.
20. Spec has a valid %clean section.
21. Macro use consistent.
22. Package contains code and (I believe) permissable content.
23. Documentation is minimal enough...
24. ...and shouldn't affect runtime.
25. n/a, no header files or static libraries.
26. n/a, no .pc files.
27. n/a, no library files.
28. n/a, no -devel subpackage.
29. n/a, no .la files.
30. Desktop file appears to get installed in the prescribed manner.
31. Nothing else owns anything with the name "twinkle," so I think we're safe.
32. Release tag contains %{?dist}, unsurprisingly.
33. n/a, already contains COPYING.
34. n/a, translations probably not available.
35. Package builds in Plague/Mock.
36. I can't verify x86_64, but it builds fine on i386 and ppc.  Kevin said it
built under x86_64, though, so we should be good there.
37. Works beautifully under i386.  (I don't have any other test environments.)
38. n/a, no scriptlets.
39. n/a, no subpackages.

Sorry this review took as long as it did; I actually wanted to test the
application to see how well it would work for my own use.  It communicated just
fine with another (closed-source) SIP softphone.

Unless the rpmlint/debuginfo warnings are of particular concern (anyone?), I
think we can probably call twinkle APPROVED.

Thanks very much for the review... 

Imported and built for devel. 
FC-5 branch requested, will build that later when the branch appears. 

 18670 (twinkle): Build on target fedora-development-extras succeeded.
     Build logs may be found at http://buildsys.fedoraproject.org/logs/fedora-
development-extras/18670-twinkle-0.8.1-2.fc6/

Please do more reviews! We need more reviewers.