Some minor comments on this patch, other than that LGTM.


http://codereview.appspot.com/186167/diff/1/4
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/4#newcode199
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/JsonQueryServlet.java:199:

this segment of the patch looks like a white space change?

http://codereview.appspot.com/186167/diff/1/5
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java
(left):

http://codereview.appspot.com/186167/diff/1/5#oldcode55
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/RedirectServlet.java:55:
* @scr.component immediate="true" metatype="no"
was there a reason to remove metatype ?

http://codereview.appspot.com/186167/diff/1/2
File
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java
(right):

http://codereview.appspot.com/186167/diff/1/2#newcode110
bundles/servlets/get/src/main/java/org/apache/sling/servlets/get/impl/helpers/JsonRendererServlet.java:110:
allowedLevel = Integer.parseInt(e.getMessage());
since you have a specific exception why not have an int in the exception
rather than parsing ?

http://codereview.appspot.com/186167

Reply via email to