Thanks for the review. I'll probably submit later today pending any other feedback.
http://codereview.appspot.com/13047/diff/1013/1029 File java/common/src/main/java/org/apache/shindig/config/ContainerConfigELResolver.java (right): http://codereview.appspot.com/13047/diff/1013/1029#newcode36 Line 36: public static final String CURRENT_CONFIG_KEY = "current"; On 2009/02/05 16:49:26, levik wrote:
Templates spec mandates a generic key for referring to the current
data context
to be "Cur" - should probably make this consistent.
Done. http://codereview.appspot.com/13047/diff/1013/1026 File java/common/src/main/java/org/apache/shindig/expressions/JsonELResolver.java (right): http://codereview.appspot.com/13047/diff/1013/1026#newcode117 Line 117: if (property instanceof Number) { On 2009/02/05 02:11:00, louiscryan wrote:
any need to check for property == null? You use String.valueOf which
insulates
you in other places. Could mess with instanceof check.
Done; there'll still be NumberFormatException wrapped into an ELException, but that's much better than a NullPointerException. http://codereview.appspot.com/13047/diff/1013/1039 File java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetELResolver.java (right): http://codereview.appspot.com/13047/diff/1013/1039#newcode91 Line 91: return ImmutableMap.of(); On 2009/02/05 02:11:00, louiscryan wrote:
Any reason not to use empty json object?
EL doesn't really care; and this doesn't allocate anything. Added a comment to that effect. http://codereview.appspot.com/13047/diff/1013/1034 File java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java (right): http://codereview.appspot.com/13047/diff/1013/1034#newcode106 Line 106: // List<Object> aliases = Arrays.<Object>asList("some-alias", "alias"); On 2009/02/05 02:11:00, louiscryan wrote:
cleanup?
Oops, thanks. http://codereview.appspot.com/13047/diff/1013/1040 File pom.xml (right): http://codereview.appspot.com/13047/diff/1013/1040#newcode232 Line 232: </roles> On 2009/02/05 02:11:00, louiscryan wrote:
separate patch.
Done. http://codereview.appspot.com/13047
