> On July 18, 2013, 8:48 p.m., Ryan Baxter wrote: > > 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
I have not (yet). The code was originally designed to be able to function separately from Shindig as I did not want to fork the Shindig 2.0.x branch on my own. It's a pity making a proper patch did not occur to me earlier. I took a quick look at HttpFetcher and it should be fairly easy to implement. As for the rest, I have very limited experience with Shindig 2.0.x myself so would appreciate any pointers. Regarding the code cleanup... I should have checked for trailing whitespaces before I created the diff, my bad. Corrected immediately with a simple vim command. Thanks for the pointer regarding Eclipse; I (think) I managed to get the code formatted properly now. Earlier I tried to import the code as a project, but failed. Now I only opened the individual code files and formatted them, but this seems to be sufficient. - Charl ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12647/#review23438 ----------------------------------------------------------- On July 22, 2013, 12:19 p.m., Charl van Niekerk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12647/ > ----------------------------------------------------------- > > (Updated July 22, 2013, 12:19 p.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 > 1505652 > > Diff: https://reviews.apache.org/r/12647/diff/ > > > Testing > ------- > > > Thanks, > > Charl van Niekerk > >
