Allow override for JS loading mechanism

Re-structured logic and cleaned-up test-only stuff.

[johnfargo, 2011-04-12 01:04:57]

small bits.

http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
(right):

http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java:71:
bindConstant().annotatedWith(Names.named("shindig.jsload.require-onload")).to(true);
consider naming to require-onload-with-jsload

http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java
(right):

http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java:53:
@Named("shindig.jsload.require-onload") boolean requireOnload) {
for more flexibility, could make this Provider<Boolean>

http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java:103:
String createJsloadScript(Uri uri) {
sounds like this might be the only thing we need in certain situations to be
protected. I'd make it so as well.

[mhermanto, 2011-04-12 01:54:40]

On Mon, Apr 11, 2011 at 6:04 PM, <johnfargo@gmail.com> wrote:

> small bits.
>
>
>
>
http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
> File
>
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
> (right):
>
>
>
http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java:71:
>
>
>
bindConstant().annotatedWith(Names.named("shindig.jsload.require-onload")).to(true);
> consider naming to require-onload-with-jsload
>

Done.

>
>
>
http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
> File
>
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java
> (right):
>
>
>
http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java:53:
>
> @Named("shindig.jsload.require-onload") boolean requireOnload) {
> for more flexibility, could make this Provider<Boolean>
>

Prolly not now, mimicking jsloadTtlSecs.


>
>
http://codereview.appspot.com/4388053/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsLoadProcessor.java:103:
> String createJsloadScript(Uri uri) {
> sounds like this might be the only thing we need in certain situations
> to be protected. I'd make it so as well.


Done.


>
> http://codereview.appspot.com/4388053/
>

Re-uploading patch.

[mhermanto, 2011-04-12 01:55:09]

Addressing John's comments

[fargo, 2011-04-12 02:24:17]

LGTM

On Mon, Apr 11, 2011 at 6:55 PM, <mhermanto@gmail.com> wrote:

> Addressing John's comments
>
>
> http://codereview.appspot.com/4388053/
>