Filter-out to-be-marked-cajole FeatureBundle

To further reduce JS size based on request. This will affect JS serving based on the new pipeline, ie: not GGS-served JS immediately.

[johnfargo, 2011-03-28 23:24:13]

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

http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:53:
Map<String, String> attribs = feature.getAttribs();
I would expect cajole to be an attribute provided on a FeatureResource rather
than a Feature Bundle. What's your thinking about the syntax used for this?

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

http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsContent.java:33:
this(content, source, null);
preference for some default value provided here

[mhermanto, 2011-03-29 01:54:41]

Addressing John's comments

[mhermanto, 2011-03-29 01:55:19]

On Mon, Mar 28, 2011 at 4:24 PM, <johnfargo@gmail.com> wrote:

>
>
>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
> File
>
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
> (right):
>
>
>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:53:
> Map<String, String> attribs = feature.getAttribs();
> I would expect cajole to be an attribute provided on a FeatureResource
> rather than a Feature Bundle. What's your thinking about the syntax used
> for this?
>

You're right. Fixed.


>
>
>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
> File
>
> java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsContent.java
> (right):
>
>
>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
> java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsContent.java:33:
> this(content, source, null);
> preference for some default value provided here


Put back.


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

[johnfargo, 2011-03-29 02:01:29]

LGTM

On Mon, Mar 28, 2011 at 6:54 PM, Michael Hermanto <mhermanto@gmail.com>wrote:

>
>
> On Mon, Mar 28, 2011 at 4:24 PM, <johnfargo@gmail.com> wrote:
>
>>
>>
>>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
>> File
>>
>>
>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
>> (right):
>>
>>
>>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
>>
>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:53:
>> Map<String, String> attribs = feature.getAttribs();
>> I would expect cajole to be an attribute provided on a FeatureResource
>> rather than a Feature Bundle. What's your thinking about the syntax used
>> for this?
>>
>
> You're right. Fixed.
>
>
>>
>>
>>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
>> File
>>
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsContent.java
>> (right):
>>
>>
>>
http://codereview.appspot.com/4316046/diff/1/java/gadgets/src/main/java/org/a...
>>
>> java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsContent.java:33:
>> this(content, source, null);
>> preference for some default value provided here
>
>
> Put back.
>
>
>>
>>
>> http://codereview.appspot.com/4316046/
>>
>
>

[Jasvir, 2011-03-29 20:34:03]

http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
(right):

http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:33:
static final String CAJOLE_ATTRIB_KEY = "cajole";
Should this be caja rather than cajole?  The url param key is caja.

[mhermanto, 2011-03-29 20:45:39]

On Tue, Mar 29, 2011 at 1:34 PM, <jasvir@gmail.com> wrote:

>
>
>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>
> File
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
> (right):
>
>
>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:33:
> static final String CAJOLE_ATTRIB_KEY = "cajole";
> Should this be caja rather than cajole?  The url param key is caja.
>
>
Works with me. I'll prepare a follow-up CL.



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

[johnfargo, 2011-03-29 21:52:11]

Let's the param key via UriCommon.Param.

On Tue, Mar 29, 2011 at 1:45 PM, Michael Hermanto <mhermanto@gmail.com>wrote:

>
>
> On Tue, Mar 29, 2011 at 1:34 PM, <jasvir@gmail.com> wrote:
>
>>
>>
>>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>>
>> File
>>
>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
>> (right):
>>
>>
>>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>>
>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:33:
>> static final String CAJOLE_ATTRIB_KEY = "cajole";
>> Should this be caja rather than cajole?  The url param key is caja.
>>
>>
> Works with me. I'll prepare a follow-up CL.
>
>
>
>>
>> http://codereview.appspot.com/4316046/
>>
>
>

[mhermanto, 2011-03-29 21:53:08]

Update patch, addressing Jasvir comments.

[mhermanto, 2011-03-29 23:10:45]

Addressing John's comments.

[mhermanto, 2011-03-29 23:16:53]

Hm, the class name suggests it's Uri param. I suppose it doesn't
hurt. Committed anyway r1086800.


On Tue, Mar 29, 2011 at 2:52 PM, John Hjelmstad <johnfargo@gmail.com> wrote:

> Let's the param key via UriCommon.Param.
>
>
> On Tue, Mar 29, 2011 at 1:45 PM, Michael Hermanto <mhermanto@gmail.com>wrote:
>
>>
>>
>> On Tue, Mar 29, 2011 at 1:34 PM, <jasvir@gmail.com> wrote:
>>
>>>
>>>
>>>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>>>
>>> File
>>>
>>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java
>>> (right):
>>>
>>>
>>>
http://codereview.appspot.com/4316046/diff/6001/java/gadgets/src/main/java/or...
>>>
>>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CajoleJsSubtractingProcessor.java:33:
>>> static final String CAJOLE_ATTRIB_KEY = "cajole";
>>> Should this be caja rather than cajole?  The url param key is caja.
>>>
>>>
>> Works with me. I'll prepare a follow-up CL.
>>
>>
>>
>>>
>>> http://codereview.appspot.com/4316046/
>>>
>>
>>
>