dev@javaserverfaces.java.net

Re: Call for code review for JAVASERVERFACES-3334

From: zhijun Ren <ren.zhijun_at_oracle.com>
Date: Mon, 11 Aug 2014 09:49:19 +0800

Hi Manfred,

Valuable comments for me, those are rules for me to abide by in the
future, thanks.

Fixed and attach the new change bundle for further review.

BR,
Zhijun


On 8/8/14, 21:30, manfred riem wrote:
> Hi Zhijun,
>
> Sorry to ask for another change but can you change the following?
>
> The <finalName>section and the glassfish-web.xml need to match the
> <name> section.
>
> Eg. <finalName>jsf-glassfish-AnnotationValidatorTest</finalName>
> should be
> <finalName>test-glassfish-facelets-core</finalName>
>
> And the glassfish-web.xml should contain /test-glassfish-facelets-core
>
> And similarly for the second project, but then for weblogic (with a
> weblogic.xml)
>
> Thanks!
> Manfred
>
> On 8/8/14, 3:49 AM, zhijun Ren wrote:
>> Hi Manfred,
>>
>> Complete to change to use the new style testing. new change bundle
>> attached here.
>>
>> I have a question about module test/weblogic, the projects under it
>> were never executed by any Hudson jobs, right?
>>
>> BR,
>> Zhijun
>>
>>
>>
>> On 8/8/14, 3:16, manfred riem wrote:
>>> Hi Zhijun,
>>>
>>> Almost there, please remove the usage of HtmlUnitFacesITCase and use
>>> the new style testing that
>>> uses annotations.
>>>
>>> Thanks!
>>> Manfred
>>>
>>> On 8/7/14, 12:55 AM, zhijun Ren wrote:
>>>> Hi Manfred,
>>>>
>>>> Thanks for your comments. I have fixed them, attach the updated
>>>> changebundle and newfiles here for your double check.
>>>>
>>>> BR,
>>>> Zhijun
>>>>
>>>> On 8/7/14, 1:55, manfred riem wrote:
>>>>> Hi Zhijun,
>>>>>
>>>>> Can you implement the following changes and send out another
>>>>> changebundle?
>>>>>
>>>>> 1. Please order the modules section in the POMs alphabetically.
>>>>> 2. Please do not include new dependencies in the dependency section.
>>>>> 3. Please make sure the web.xml files contain the ${webapp.xxxx}
>>>>> parameters,
>>>>>
>>>>> <context-param>
>>>>> <param-name>javax.faces.PROJECT_STAGE</param-name>
>>>>> <param-value>${webapp.projectStage}</param-value>
>>>>> </context-param>
>>>>> <context-param>
>>>>> <param-name>javax.faces.PARTIAL_STATE_SAVING</param-name>
>>>>> <param-value>${webapp.partialStateSaving}</param-value>
>>>>> </context-param>
>>>>> <context-param>
>>>>> <param-name>javax.faces.STATE_SAVING_METHOD</param-name>
>>>>> <param-value>${webapp.stateSavingMethod}</param-value>
>>>>> </context-param>
>>>>>
>>>>> 4. Change AnnotatedComponentsITCase to AnnotatedComponentIT and use
>>>>> the new testing pattern, see any of the projects for an
>>>>> example on that, or
>>>>> if you need help let me know.
>>>>>
>>>>> 5. Fix the package names to reflect which project they are in
>>>>> (this makes it
>>>>> easier to figure out which project a given class belongs to
>>>>> when you look
>>>>> at the test report).
>>>>>
>>>>> Eg.
>>>>> com.sun.faces.test.weblogic.wls1214.facelets.core.AnnotationTestBean
>>>>> for the code in test/weblogic/wls1214/facelets/core
>>>>>
>>>>> Do that both for the tests and the managed beans.
>>>>>
>>>>> 6. When using HtmlUnit please do not use the asText() method, but
>>>>> always use asXml().
>>>>>
>>>>> Thanks!
>>>>> Manfred
>>>>>
>>>>> On 8/6/14, 3:16 AM, zhijun Ren wrote:
>>>>>> Hi Manfred and Ed,
>>>>>>
>>>>>> Please help to do code review for my change for JIRA3334,
>>>>>> attached the changebundle.txt and the newfiles.zip.
>>>>>>
>>>>>> Main changes:
>>>>>> 1. Only copy the AnnotatedValidatorNoValue Test logic by rewrite
>>>>>> the web project and related test classes;
>>>>>> 2. Uncomment the @FacesValidator in AnnotatedValidatorNoValue;
>>>>>> 3. The tests are in test/glassfish and test/weblogic/wls11214 now;
>>>>>>
>>>>>> Thanks for any comments.
>>>>>>
>>>>>> Zhijun