----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12647/#review23438 -----------------------------------------------------------
In general I understand what is happening here but I feel like there should be more reuse of the code from Shindig. I am less familiar with then 2.0.x stream so maybe some of this doesn't exist there.... One place where I would expect some reuse is when you are fetching the gadget xml. At the very least I would expect you could use the HttpFetcher class that is part of Shindig instead of using the HttpClient directly. However I feel like there are probably classes at a higher level that will fetch, and parse the gadget XML so your code does not have to do that either. Have you looked around in Shindig to see if you can reuse any of that? General code cleanup comment... Anywhere you see red in the diff on review board mean there are white spaces there, you should remove that. You should probably import the Eclipse code formatting templates from trunk https://svn.apache.org/repos/asf/shindig/trunk/etc/eclipse/. There are ones for the Java and Javascript editors in Eclipse. I found this simple article that gives some details on how to import them http://www.avajava.com/tutorials/lessons/how-do-i-share-my-code-formatting-settings-with-another-user.html http://svn.apache.org/repos/asf/shindig/branches/2.0.x/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetSettingsServlet.java <https://reviews.apache.org/r/12647/#comment47313> Documentation of what the request URL looks like and which parameters are part of the URL would be helpful. - Ryan Baxter On July 17, 2013, 10:05 a.m., Charl van Niekerk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12647/ > ----------------------------------------------------------- > > (Updated July 17, 2013, 10:05 a.m.) > > > Review request for shindig and Ryan Baxter. > > > Bugs: SHINDIG-1188 > https://issues.apache.org/jira/browse/SHINDIG-1188 > > > Repository: shindig > > > Description > ------- > > In Shindig 2.0.x there is a JavaScript that is being loaded from Google. > iGoogle is shutting down in November and the expectation is that this > JavaScript will no longer be available. > > This patch contains a servlet that is supposed to replace iGoogle's > JavaScript by implementing the equivalent functionality. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/branches/2.0.x/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSettingsException.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/branches/2.0.x/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/GadgetSettings.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/branches/2.0.x/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetSettingsServlet.java > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/branches/2.0.x/java/server/src/main/webapp/WEB-INF/web.xml > 1504060 > > Diff: https://reviews.apache.org/r/12647/diff/ > > > Testing > ------- > > > Thanks, > > Charl van Niekerk > >
