dev@javaserverfaces.java.net

Re: Call for review for JAVASERVERFACES-3396

From: zhijun Ren <ren.zhijun_at_oracle.com>
Date: Tue, 09 Sep 2014 13:10:15 +0800

Hi Ed,

Actually, some of those changes, e.g., property change and '+', were
generated by 'svn merge', svn can handle these when 'svn commit'.

I do think 'svn merge' can save time for us because it do things
automatically and has other benefits, for example, it handles the
conflict case.

Anyway, I have changed according to your requirement and attached the
new change bundles to the issue.

Please review again.

BR,
Zhijun



On 9/8/14, 23:32, Edward Burns wrote:
> Thanks for this, Zhijun,
>
> Bottom line, In light of these problems, please submit another
> changebundle for review. Details below.
>
>>>>>> On Fri, 05 Sep 2014 09:58:06 +0800, zhijun Ren<ren.zhijun_at_oracle.com> said:
> Z> Please help to review, change bundle attached.
>
> I'll review it inline, but in the future, please attach it to the
> issue. If you want to *also* attach it to the email, that's up to you.
> But we like to have it on the issue.
>
> Index: .
>
>> Property changes on: .
> Please do "svn revert ." from the top level to avoid committing this
> property change. Sometimes svn makes property changes when doing merges
> and the must not be committed.
>
>
> Index: jsf-ri/src/main/java/com/sun/faces/renderkit/ServerSideStateHelper.java
>
> Looks good.
>
>
> Index: test/servlet30/pom.xml
>
> Looks good
>
> Index: test/servlet30/stateSaving/pom.xml
> ===================================================================
> --- test/servlet30/stateSaving/pom.xml (working copy)
> +++ test/servlet30/stateSaving/pom.xml (working copy)
> @@ -47,7 +47,7 @@
> <parent>
> <groupId>com.sun.faces.test.servlet30</groupId>
> <artifactId>pom</artifactId>
> -<version>2.2.9-SNAPSHOT</version>
> +<version>2.3.0-m01-SNAPSHOT</version>
> </parent>
> <groupId>com.sun.faces.test.servlet30.statesaving</groupId>
> <artifactId>pom</artifactId>
>
> DO NOT COMMIT without first stripping off the "+". You can save aside
> the diff, svn revert the file, re-apply the diff with patch to strip off
> the "+".
>
> Index: test/servlet30/stateSaving/viewExpiredException/pom.xml
> ===================================================================
> --- test/servlet30/stateSaving/viewExpiredException/pom.xml (working copy)
> +++ test/servlet30/stateSaving/viewExpiredException/pom.xml (working copy)
> @@ -45,11 +45,11 @@
> <parent>
> <groupId>com.sun.faces.test.servlet30.statesaving</groupId>
> <artifactId>pom</artifactId>
> -<version>2.2.9-SNAPSHOT</version>
> +<version>2.3.0-m01-SNAPSHOT</version>
> </parent>
> <groupId>com.sun.faces.test.servlet30.statesaving</groupId>
> <artifactId>viewexpiredexception</artifactId>
> -<version>2.2.9-SNAPSHOT</version>
> +<version>2.3.0-m01-SNAPSHOT</version>
> <packaging>war</packaging>
> <name>Mojarra ${project.version} - Test - Servlet 3.0 - State Saving - ViewExpireException</name>
>
> Change looks good, but strip off the "+" as above.
>
> A + test/servlet30/stateSaving
>
> I should see many more files than just the directory. Looking at 3359,
> I see
>
> 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
>
> These should be svn added here, again making sure to strip off the "+".
>
> In light of these problems, please submit another changebundle for
> review.
>
> Thanks for your persistence.
>
> Ed
>