From: Tas S. <ta...@go...> - 2009-10-23 14:03:19
|
Hi Marc, 2009/10/23 Marc 'BlackJack' Rintsch <ma...@ri...> > Tas Sóti wrote: > > Now I've found that I do not need WritableList and data binding at all, > > because the number of entries in the Roster does not change while the > > InvitationWizard is opened (only the presences...). The entries only > change > > if a new entry is added or an old one is deleted... if you open the > > InvitationWizard after that, the TableViewer gets the actual list > > (Roster.getEntries()), so it works fine without data binding. > > I had a cursory look at the patch, because I found it too big to try to > understand it without a walk through. > > Some observations: > > `EditorAPI.saveProject()`: New `confirm` argument is undocumented. > > `XMPPChatTransmitter.receiveUserListConfirmation`: I would change the > ``do``/``while`` into a ``while`` loop. When `fromUsers` is empty, the > loop is executed at least once now. > > The comment on `collector.nextResult()` is wrong -- it's milliseconds. > > The first `debug()` should be a `warn()` IMHO, because it is an > unexpected event. > > I don't like the method name `sendMessageToArbitraryUser()`. > "Arbitrary" means "frei wählbar" in German, but also "beliebig", > "willkürlich", "unbegründet". Wouldn't `sendMessageToUser()` be enough? > > ---- > > Not sure if I could give a +0 from such quick look at the patch. But we > need this patch because without it, the invitation does not work at the > moment, because of already committed changes to the > `XMPPChatTransmitter` the old invitation code has problems with. > I'll have a look at these code segments as soon as possible, but I'm at work right now. We reviewed this patch with Henning and Sándor...last week!?. I'm waiting for the after review from Henning after some minor changes, but he seems to have a lots to do. What can I do? :) Best, Tas |