Hi Hanspeter,
thank you for the fix, as I am kind of the Reporter of 938, I am fine with closing #938 as finally resolved :)
The latest patch you attached still does have calls to 'super', but in your commit to mojarra these super s have been exchanged by the correct call to getWrapped(), right?
I am not sure about the process with patch-files, but I personally think it is a good idea to have the latest / correct patch attached to the issue. But I think this is up to Ed how to handle this.
Thanks again.
Regards,
Imre
On 19.05.2011, at 15:27, Dünnenberger Hanspeter (KSXK 43) wrote:
> 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
- application/pkcs7-signature attachment: smime.p7s