Ed Burns wrote:
>>>>>>On Thu, 11 Nov 2004 10:04:37 -0500, Roger Kitain <Roger.Kitain_at_Sun.COM> said:
>>>>>>
>>>>>>
>
>RK> Please review the change bundle at:
>RK> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=21
>
>RK> Note: This is only a partial checkin for ConverterTag/ValidatorTag in api.
>RK> The rest of the checkin will be done for some core tags (RI) and TLV
>RK> modifications.
>
>1. New Messages
>
>First, I would have preferred to have done the new messages, some of
>which appear to have nothing to do with 21-MethodBindings, in a separate
>checkin. You can leave it as is now, but in the future, I like to keep
>the checkins as tightly focused and cohesive as possible.
>
>Second, all of these must be added to section 2.5.2.4 in the spec PDF
>document.
>
right.
>
>+ public static final String INVALID_EXPRESSION_MESSAGE_ID =
>"javax.faces.webapp.INVALID_EXPRESSION";
>
>Third, this should be javax.el.INVALID_EXPRESSION. We can do it this
>way now since it's only a message key, and a new one at that.
>
made it javax.face.el
>
>+ public static final String COMPONENT_FROM_TAG_ERROR_MESSAGE_ID =
>"javax.faces.webapp.COMPONENT_FROM_TAG_ERROR";
>+ public static final String NOT_NESTED_IN_TYPE_TAG_ERROR_MESSAGE_ID =
>"javax.faces.webapp.NOT_NESTED_IN_TYPE_TAG_ERROR";
>+ public static final String NOT_NESTED_IN_FACES_TAG_ERROR_MESSAGE_ID =
>"javax.faces.webapp.NOT_NESTED_IN_FACES_TAG_ERROR";
>+ public static final String CANT_CREATE_CLASS_ID =
>"javax.faces.webapp.CANT_CREATE_CLASS";
>
>2. Error spacing
>
>+ if (converterId != null) {
>+ if (converterError != null) {
>+ converterError += "or " + converterId;
>+ } else {
>+ converterError = converterId;
>+ }
>
>Don't you need a " " before the "or "?
>
fixed.
>
>3. null checking
>
>In the new createConverter(), you say:
>
>+ converterIdVal = (String) idBinding.getValue(context);
>+ }
>+ converter =
>+ context.getApplication().createConverter(converterIdVal);
>
>If idBinding.getValue() return null, converterIdVal will be null, and
>you'd get an NPE. Is that what we want? It may be, I just wanted your
>opinion on it. I'm ok leaving it as is, since I suspect that's how it
>was to begin with.
>
left it for now (was like this before).
>
>4. I18N messages. Please add these new messages to all the locales.
>For non-english, just use the english versions, and our I18N team will
>localize them for us.
>
Done.
>
>Make these changes and you have r=edburns to go through the checkin
>process.
>
>Ed
>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>
>
>