CMakeLists.txt (Diff revision 1)

Show all issues

rename-like-the-others

1. 

   Rohan Garg Oct. 28, 2012, 10:15 a.m.

   Righto

conferenceauth-observer.cpp (Diff revision 1)

Show all issues

Don't do this.

Instead

foreach (Tp::ChannelPtr channels) {
  if (channel->hasInterface(...)  {
     doStuff();
   }
}

I don't fully understand when you get two channels at once, but I think it is possible. Otherwise it wouldn't be a list in the first place

1. 

   Rohan Garg Oct. 28, 2012, 10:15 a.m.

   Yeah, already in progress, saw it right after I posted the review.
   As for multiple channels, I /think/ this might be a usecase when I implement auto-joining channels and alot of the channels have a password interface

conferenceauth-observer.cpp (Diff revision 1)

Show all issues

I don't see why we need to wait until the passwordOp is ready order to complete the methodInvocationContext.

I /think/ this can be moved into the observeChannels which will make launching channels quicker. I may be wrong though.

1. 

   Rohan Garg Oct. 28, 2012, 10:15 a.m.

   Will fix this once I test this out.

Brilliant. That's very nice and tidy.

main.cpp (Diff revision 4)

Show all issues

You don't use this. I would suggest removing it. and putting the warning in the if statement instead.

This review has been submitted with commit 0ecb0a62cfa580a399e3928f13b04edcc42a6cc3 by Rohan Garg to branch master.