Hi Adam, thanks very much for reviewing my patches so promptly. Here are my
comments to your comments:
ADFFACES-475:
- If the method name is immaterial at runtime then the only change for the
companion MYFACES-1599 JIRA would be to update the return value which I've done
with the patch attached to MYFACES-1599. If no one objects I think I should just
close ADFFACES-475.
ADFFACES-476:
- I really like that solution. I shall provide you with another patch (assuming
I can discern what the spec components are).
ADFFACES-477:
- To be honest I was a bit hesitate to alter the current "CAN_COERCE" check
since I was not fully certain of the implications. If there are none, then it
seems the _CAN_COERCE map can be removed as well since that is the only place it
is used. If that is the case I shall provide you with another patch.
Thanks,
Tim McConnell
Adam Winer wrote:
I'm checking out the following three issues, and have comments on the
patches for each.
https://issues.apache.org/jira/browse/ADFFACES-475:
- Changes the deferred-method method name from "myMethod() to action()".
I don't get it. The method name in the TLD has zero runtime meaning.
The spec
should not require any particular name, and it'd be a spec bug if it does
(or, at least, the spec should clarify). If we're going to change
anything, I'd
rather change the method name from generically being "myMethod()" to always
being the TLD attribute name (e.g., "action()", "beforePhase()", etc.).
But, mostly I'm inclined to push back on the spec.
https://issues.apache.org/jira/browse/ADFFACES-476:
- Changes rtexprvalue on "id" from true to false
Supporting rtexprvalue on "id" is useful for component libs. I think this
should be a configurable option on the plugin (by default, on) that we
can turn off for the spec components but keep on for Trinidad +
Tomahawk.
https://issues.apache.org/jira/browse/ADFFACES-477
- Changes the deferred-value type on "converter" and "binding" from
java.lang.Object to the appropriate type
I'm OK with this one, though I don't see any RT benefits. There's
some small tooling benefit to the change. I think the simplest change
to make is removing the current "CAN_COERCE" check in the code,
instead of hardcoding "binding" and "converter" as special attrs.
-- Adam
