-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3882/#review5271
-----------------------------------------------------------

Ship it!


Just left a bunch of minor comments. 


src/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantController.java
<https://reviews.apache.org/r/3882/#comment11517>

    Please rename participantList to "participants" as it's not a java.util.List



src/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantController.java
<https://reviews.apache.org/r/3882/#comment11518>

    Why not use here enhanced (java 5) for loop?



test/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantControllerTest.java
<https://reviews.apache.org/r/3882/#comment11516>

    Copyright 2010 Google Inc.
    ->
    Copyright 2012 Apache Wave



test/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantControllerTest.java
<https://reviews.apache.org/r/3882/#comment11515>

    Please remove empty line


- Yuri


On 2012-02-18 12:54:05, rocklund wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3882/
> -----------------------------------------------------------
> 
> (Updated 2012-02-18 12:54:05)
> 
> 
> Review request for wave.
> 
> 
> Summary
> -------
> 
> Implemented as suggested in the jira-issue as a client side implementation in 
> ParticipantController.java. I selected the separator for the multiple 
> participant list to be semi-colon(;).
> 
> 
> This addresses bug wave-317.
>     https://issues.apache.org/jira/browse/wave-317
> 
> 
> Diffs
> -----
> 
>   
> src/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantController.java
>  ec9e25a 
>   
> test/org/waveprotocol/wave/client/wavepanel/impl/edit/ParticipantControllerTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3882/diff
> 
> 
> Testing
> -------
> 
> Compiled and run.
> 
> * Tested to add single as well as multiple participants
> * Tested to add participants both with and without @localhost
> * Tested to add invalid participants
> 
> 
> Thanks,
> 
> rocklund
> 
>

Reply via email to