> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.cpp, lines 466-469
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75962#file75962line466>
> >
> >     1) StatusArea does not implement the Error message type, so these are 
> > essentially no-op. I thought this was why you wanted the KMessageWidget...
> >     
> >     2) The messages are not very clear. You need to tell the user that the 
> > call is on hold, somehow. Especially in the case of ResourceNotAvailable, 
> > the string should be something like "Failed to put the call off hold: Some 
> > devices are not available", because this can only happen if you are trying 
> > to put the call off hold and the backend fails to open some input/output 
> > device (camera, sound card).

1) I planned to do that as a separate patch altogether, or I could add it to 
this one ... your choice :)

2) Will fix


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.cpp, lines 477-478
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75962#file75962line477>
> >
> >     This cannot happen in StateUnheld. It should also be handled as unknown 
> > error.

Why not? Let's say the user hold's a call and a couple of seconds later the 
driver responsible for Video/Audio capture crashes or the wires are not 
securely connected causing a input error. Won't the telepathy backend reply 
with a  Tp::LocalHoldStateReasonResourceNotAvailable error?


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.ui, lines 39-50
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75963#file75963line39>
> >
> >     Afaict, the errorWidget here is on the same page as the user avatar 
> > label, which means that if we are on a video call, this widget will not 
> > appear...

Derp. I'll fix this one ...


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/main.cpp, line 67
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75964#file75964line67>
> >
> >     This will cause a failure if the hold interface is not available in the 
> > call, but I think both gabble and rakia implement it so I think we can get 
> > away with it for now

Is it possible to query the backend if a particular feature is available? If 
yes, why not do it for all the features we add?


- Rohan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105829/#review16852
-----------------------------------------------------------


On Aug. 3, 2012, 9:56 p.m., Rohan Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105829/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2012, 9:56 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Enables call holding in ktp-call-ui
> 
> 
> Diffs
> -----
> 
>   src/main.cpp 4ab3a23 
>   src/call-window.ui 14144cc 
>   src/call-window.h d8741c3 
>   src/call-window.cpp e4aed4d 
>   src/call-manager.cpp df6e701 
> 
> Diff: http://git.reviewboard.kde.org/r/105829/diff/
> 
> 
> Testing
> -------
> 
> Successfully held and unheld calls using ktp-call-ui
> 
> 
> Thanks,
> 
> Rohan Garg
> 
>

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to