dev@javaserverfaces.java.net

RE: Review: 917

From: KSXK 43 <Dünnenberger>
Date: Thu, 19 May 2011 15:27:09 +0200

Hi Ed and dev-list.

Thanks for clarification about this marking thing.

Maybe it would be good to describe that in Mojarra/developers-info.txt (or similar name). I realize there is already a bunch of files with different kind of information for developers, maybe all that information can be reorganized/updated/combined into one developers info. In CS-JSF I switched to a tiddly-wiki (one HTML file wiki) for the developers info, because that one-page wiki allows linking.

I was also checking for some standard/recommended format of the commit comment - but could hardly find one. So I tried my best - please look at the commit comment. I realized there where many commit comments containing a SECTION: Modified files - are you generating this somehow?

So, I commited this a few minutes ago. I guess I should also update the issue in JIRA. Since there are two related issues, which are solved with this change too - should I mark all of them as resolved or do you prefer to mark some as duplicate?


regards
Hanspeter


 

-----Original Message-----
From: Ed Burns [mailto:edward.burns_at_oracle.com]
Sent: Wednesday, May 18, 2011 3:47 PM
To: Mojarra dev list
Cc: Dünnenberger Hanspeter (KSXK 43)
Subject: Review: 917

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