[Bug 28413] Gabble DTMF

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28413

Jonny Lamb <jonny.lamb at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review+, minor questions

--- Comment #10 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2010-10-22 05:09:25 PDT ---
Now some comments on your dtmf branch:

 * gabble_dtmf_player_play has loads of g_return_val_if_fail to check for bad
   arguments, but doesn't check whether self is what it should be, so you could
   easily pass NULL into it, etc.

 * Just a nitpick, but the sig_id_started_tone IDs are fine, but just different
   from (I believe) everywhere in gabble and tp-glib. Why didn't you do the
   standard thing of creating a signals array and enum?

The rest of the branch looks totally fine, and I can't see why GabbleDTMFPlayer
can't become TpDTMFPlayer.

Also, it turns out you fix all my comments about your dtmf-from-the-past branch
I gave above in your dtmf branch, so there's no point in wasting time and
trying to fix them.

I have already ++'d your spec branch, so I say merge that, and merge your dtmf
branch. It's up to you what you want to do about the new spec stuff -- either
merge your dtmf branch now and leave the commented out parts commented out, or
wait until a new spec release, then new tp-glib release, and uncomment them out
before merging the dtmf branch.

Way to go!

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.