You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Yao Zhang <ya...@gmail.com> on 2012/04/09 08:43:41 UTC
Review Request: Need to support query parameter for Social REST API
(continue li's patch).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/
-----------------------------------------------------------
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
-----
Diff: https://reviews.apache.org/r/4680/diff
Testing
-------
done
Thanks,
Yao
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/#review6825
-----------------------------------------------------------
Code looks good. I would suggest you add some unit tests to some of the sample SPIs to make sure what you are trying to accomplish with the optional parameters will work.
- Ryan
On 2012-04-10 03:17:11, Yao Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4680/
> -----------------------------------------------------------
>
> (Updated 2012-04-10 03:17:11)
>
>
> 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 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
>
> Diff: https://reviews.apache.org/r/4680/diff
>
>
> Testing
> -------
>
> done
>
>
> Thanks,
>
> Yao
>
>
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Ryan Baxter <rb...@gmail.com>.
> On 2012-04-12 12:09:52, Ryan Baxter wrote:
> > LGTM, have you signed up for Apache/s JIRA deployment? I want to assign the JIRA to you and have you upload the final patch to the JIRA granting the ASF license before I submit the code. I tried to look for you to assign the JIRA to you but I didn't see you in the JIRA system. If you have not registered could you?
> >
> > https://issues.apache.org/jira/browse/SHINDIG
>
> Yao Zhang wrote:
> Hi Ryan,
>
> My username is yaozhang. I have upload the patch to https://issues.apache.org/jira/browse/SHINDIG-1698 and grant the ASF license.
Committed please close the review. Thanks.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/#review6873
-----------------------------------------------------------
On 2012-04-12 01:47:45, Yao Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4680/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 01:47:45)
>
>
> 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/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/CollectionOptionsTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
>
> Diff: https://reviews.apache.org/r/4680/diff
>
>
> Testing
> -------
>
> done
>
>
> Thanks,
>
> Yao
>
>
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Yao Zhang <ya...@gmail.com>.
> On 2012-04-12 12:09:52, Ryan Baxter wrote:
> > LGTM, have you signed up for Apache/s JIRA deployment? I want to assign the JIRA to you and have you upload the final patch to the JIRA granting the ASF license before I submit the code. I tried to look for you to assign the JIRA to you but I didn't see you in the JIRA system. If you have not registered could you?
> >
> > https://issues.apache.org/jira/browse/SHINDIG
Hi Ryan,
My username is yaozhang. I have upload the patch to https://issues.apache.org/jira/browse/SHINDIG-1698 and grant the ASF license.
- Yao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/#review6873
-----------------------------------------------------------
On 2012-04-12 01:47:45, Yao Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4680/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 01:47:45)
>
>
> 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/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/CollectionOptionsTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
>
> Diff: https://reviews.apache.org/r/4680/diff
>
>
> Testing
> -------
>
> done
>
>
> Thanks,
>
> Yao
>
>
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/#review6873
-----------------------------------------------------------
Ship it!
LGTM, have you signed up for Apache/s JIRA deployment? I want to assign the JIRA to you and have you upload the final patch to the JIRA granting the ASF license before I submit the code. I tried to look for you to assign the JIRA to you but I didn't see you in the JIRA system. If you have not registered could you?
https://issues.apache.org/jira/browse/SHINDIG
- Ryan
On 2012-04-12 01:47:45, Yao Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4680/
> -----------------------------------------------------------
>
> (Updated 2012-04-12 01:47:45)
>
>
> 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/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/CollectionOptionsTest.java PRE-CREATION
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
> http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
>
> Diff: https://reviews.apache.org/r/4680/diff
>
>
> Testing
> -------
>
> done
>
>
> Thanks,
>
> Yao
>
>
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Yao Zhang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/
-----------------------------------------------------------
(Updated 2012-04-12 01:47:45.851337)
Review request for shindig, Ryan Baxter, Eric Woods, and Stanton Sievers.
Changes
-------
Hi Ryan, test case is added. Please review.
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/test/java/org/apache/shindig/social/opensocial/spi/CollectionOptionsTest.java PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java 1311182
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
Diff: https://reviews.apache.org/r/4680/diff
Testing
-------
done
Thanks,
Yao
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Yao Zhang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/
-----------------------------------------------------------
(Updated 2012-04-10 03:17:11.958169)
Review request for shindig, Ryan Baxter, Eric Woods, and Stanton Sievers.
Changes
-------
Hi Ryan, I have update the patch per your comments. Thanks!
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/BaseRequestItem.java 1311182
http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/protocol/RequestItem.java 1311182
http://svn.apache.org/repos/asf/shindig/trunk/java/social-api/src/main/java/org/apache/shindig/social/opensocial/spi/CollectionOptions.java 1311182
Diff: https://reviews.apache.org/r/4680/diff
Testing
-------
done
Thanks,
Yao
Re: Review Request: Need to support query parameter for Social REST API
(continue li's patch).
Posted by Yao Zhang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4680/
-----------------------------------------------------------
(Updated 2012-04-09 06:54:36.602784)
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 (updated)
-----
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