dev@javaserverfaces.java.net

Review: 917

From: Ed Burns <edward.burns_at_oracle.com>
Date: Wed, 18 May 2011 06:47:13 -0700

Hello Hanspeter,

Thanks for submitting the patch for JAVASERVERFACES_SPEC_PUBLIC-917. I
realize these comments are very specific and annoying, but it is the
convention we've been following for years. I'd like to be consistent
where possible.

Index: jsf-api/src/main/java/javax/faces/application/package.html
===================================================================
--- jsf-api/src/main/java/javax/faces/application/package.html (revision 9050)
+++ jsf-api/src/main/java/javax/faces/application/package.html (working copy)
@@ -47,7 +47,7 @@
 <body bgcolor="white">
 
 <p><span class="changed_modified_2_0 changed_modified_2_0_rev_a
-changed_modified_2_1">APIs</span> that are used to link an application's
+changed_modified_2_1 changed_modified_2_2">APIs</span> that are used to link an application's
 business logic objects to JavaServer Faces, as well as convenient
 pluggable mechanisms to manage the execution of an application that is
 based on JavaServer Faces. The main class in this package is {_at_link
---------------------------------------------------

Because this is the first change to any class in this package in JSF
2.2, we need to flag that fact so the top level javadocs reflect the
change. What you have done here is exactly correct.

Index: jsf-api/src/main/java/javax/faces/application/ResourceWrapper.java
===================================================================
--- jsf-api/src/main/java/javax/faces/application/ResourceWrapper.java (revision 9050)
+++ jsf-api/src/main/java/javax/faces/application/ResourceWrapper.java (working copy)
@@ -49,7 +49,7 @@
 import javax.faces.context.FacesContext;
 
 /**
- * <p class="changed_added_2_0">Provides a simple implementation of
+ * <p class="changed_added_2_0 changed_modified_2_2">Provides a simple implementation of
  * {_at_link Resource} that can be subclassed by developers wishing to
  * provide specialized behavior to an existing {_at_link Resource}
  * instance. The default implementation of all methods is to call
---------------------------------------------------

This one is a little off. The class was added in 2.0, and modified in
2.2. In this case, we don't want the whole paragraph to show up orange,
just the first word. Like this:

- * <p class="changed_added_2_0"><span class="changed_modified_2_2">Provides</span> a simple implementation of

The same thing for ExternalContextWrapper.java and
PartialViewContextWrapper.java. For PartialViewContextWrapper.java, I
notice that the class, while added in JSF 2.0, was not flagged as such.
Can you also do this for that class:

Index: jsf-api/src/main/java/javax/faces/context/PartialViewContextWrapper.java
===================================================================
--- jsf-api/src/main/java/javax/faces/context/PartialViewContextWrapper.java (revision 9050)
+++ jsf-api/src/main/java/javax/faces/context/PartialViewContextWrapper.java (working copy)
@@ -46,7 +46,7 @@
 import javax.faces.event.PhaseId;
 
 /**
+ * <p class="changed_added_2_0"><span class="changed_modified_2_2">Provides</span> a simple implementation of {_at_link PartialViewContext} that can

Thank you. And with that, you have r=edburns. If I have been unclear
about any of these changes to your patch, please let me know.

I look forward to your commit!

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| homepage:               | http://ridingthecrest.com/
| 20 Business Days til JSF 2.2 Early Draft Review
| 56 Business Days til JSF 2.2 Public Review
| 144 Business Days til JSF 2.2 Proposed Final Draft