dev@javaserverfaces.java.net

Re: Call for code review for AVASERVERFACES-3359

From: zhijun Ren <ren.zhijun_at_oracle.com>
Date: Thu, 28 Aug 2014 09:52:09 +0800

Hi Manfred,

Thanks for the review, comments fixed and code commited to 2.2.x rolling:

rzhijun-mac:MOJARRA_2_2X_ROLLING zhijun$ svn commit -m
"https://java.net/jira/browse/JAVASERVERFACES-3359" --username
ren.zhijun.oracle
Sending
jsf-ri/src/main/java/com/sun/faces/renderkit/ServerSideStateHelper.java
Sending test/servlet30/pom.xml
Adding test/servlet30/stateSaving
Adding test/servlet30/stateSaving/pom.xml
Adding test/servlet30/stateSaving/viewExpiredException
Adding test/servlet30/stateSaving/viewExpiredException/nbactions.xml
Adding test/servlet30/stateSaving/viewExpiredException/pom.xml
Adding test/servlet30/stateSaving/viewExpiredException/src
Adding test/servlet30/stateSaving/viewExpiredException/src/main
Adding
test/servlet30/stateSaving/viewExpiredException/src/main/webapp
Adding
test/servlet30/stateSaving/viewExpiredException/src/main/webapp/WEB-INF
Adding
test/servlet30/stateSaving/viewExpiredException/src/main/webapp/WEB-INF/web.xml
Adding
test/servlet30/stateSaving/viewExpiredException/src/main/webapp/index.xhtml
Adding
test/servlet30/stateSaving/viewExpiredException/src/main/webapp/page2.xhtml
Adding test/servlet30/stateSaving/viewExpiredException/src/test
Adding test/servlet30/stateSaving/viewExpiredException/src/test/java
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces/test
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces/test/servlet30
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces/test/servlet30/statesaving
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces/test/servlet30/statesaving/viewexpiredexception
Adding
test/servlet30/stateSaving/viewExpiredException/src/test/java/com/sun/faces/test/servlet30/statesaving/viewexpiredexception/Issue3359IT.java
Transmitting file data .........
Committed revision 13606.


BR,
Zhijun

On 8/27/14, 22:53, manfred riem wrote:
> Hi Zhijun,
>
> Please change nbactions.xml to use
> test-servlet30-statesaving-viewexpiredexception as well,
> and the integration.url to be
> http://localhost:8080/test-servlet30-statesaving-viewexpiredexception/
>
> And make sure the copyright year has 2014 in all of the added /
> updated files.
>
> After that please go ahead and commit this to trunk
>
> Regards,
> Manfred
>
>
> On 8/27/14, 4:44 AM, zhijun Ren wrote:
>> Hi Manfred,
>>
>> Your comments fixed, change bundle attached(including
>> ServerSideStateHelper), please review.
>>
>> BR,
>> Zhijun
>>
>> On 8/27/14, 9:13, zhijun Ren wrote:
>>> Hi Manfred,
>>>
>>> For the code change for ServerSideStateHelper, I have given up my
>>> first time change, you can refer to the attached change bundle in
>>> the previous mail.
>>>
>>> BR,
>>> Zhijun
>>>
>>> On 8/26/14, 23:30, manfred riem wrote:
>>>> Hi Zhijun,
>>>>
>>>> Can you please move the testcase to
>>>> test/servlet30/stateSaving/viewExpiredException ?
>>>>
>>>> The test/agnostic directory is not to be used to add new tests.
>>>>
>>>> Also did you change the ServerSideStateHelper to have one return?
>>>> Did I miss it?
>>>>
>>>> Thanks for the great work!
>>>>
>>>> Regards,
>>>> Manfred
>>>>
>>>> On 8/25/14, 1:02 AM, zhijun Ren wrote:
>>>>> Hi Manfred,
>>>>>
>>>>> https://java.net/jira/browse/JAVASERVERFACES-3359
>>>>>
>>>>> Test case added for this bug, please help to review the updated
>>>>> code whose changed bundle attached here.
>>>>>
>>>>> Thanks,
>>>>> Zhijun
>>>>>
>>>>> On 8/19/14, 21:29, manfred riem wrote:
>>>>>> Hi Zhijun,
>>>>>>
>>>>>> Can ypu please make sure there is only one exit point (one return)?
>>>>>>
>>>>>> Also you will need to include a test in the changebundle as well.
>>>>>>
>>>>>> Thanks!
>>>>>> Manfred
>>>>>>
>>>>>> On 8/19/14, 1:03 AM, zhijun Ren wrote:
>>>>>>> Hi Manfred and Ed,
>>>>>>>
>>>>>>> https://java.net/jira/browse/JAVASERVERFACES-3359
>>>>>>> |Please refer the JIRA comments which I added for code diff and the test results.
>>>>>>>
>>>>>>> BR,
>>>>>>> Zhijun
>>>>>>> |
>>>>>>>