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



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
<https://reviews.apache.org/r/4680/#comment15064>

    Small style nit, space between Map parameters 



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
<https://reviews.apache.org/r/4680/#comment15065>

    instead of using new HashSet use Maps.newHashSet



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
<https://reviews.apache.org/r/4680/#comment15068>

    I don't understand why this can't be private static final..you are using 
package visible fields on RequestItem, you don't need an instance of it 
RequestItem.  Is it because its package visible?  
    
    I tried it locally and it worked fine. 



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
<https://reviews.apache.org/r/4680/#comment15066>

    style nit, put a space between the map parameters



http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
<https://reviews.apache.org/r/4680/#comment15067>

    Style nit put a space between the map parameters


- Ryan


On 2012-04-09 06:54:36, Yao Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4680/
> -----------------------------------------------------------
> 
> (Updated 2012-04-09 06:54:36)
> 
> 
> Review request for shindig, Ryan Baxter, Eric Woods, and Stanton Sievers.
> 
> 
> Summary
> -------
> 
> This is the patch for https://reviews.apache.org/r/3764 as xuli is not 
> working on it.
> @Ryan, I have merged line 58 and 59 of 
> /trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
>  and break line 60 into multiple lines. As predefinedParameters is using 
> RequestItem, it can not be changed to private static final.
> 
> 
> This addresses bug SHINDIG-1698.
>     https://issues.apache.org/jira/browse/SHINDIG-1698
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java
>  1311123 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java
>  1311123 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java
>  1311123 
> 
> Diff: https://reviews.apache.org/r/4680/diff
> 
> 
> Testing
> -------
> 
> done
> 
> 
> Thanks,
> 
> Yao
> 
>

Reply via email to