BugFix - optimizer config is not respecting the compression ratio specified.

Bug: OptimizerConfig is not respecting the compression ratio specified in the shindig properties, and always getting set to 0.9.

And also change the compression ratio in shindig.properties so that existing behavior will be retained.

[anupama.dutta, 2010-10-07 13:16:13]

The changes look mostly good. Could you separate this out into two changes?
The config change should be simple and should go in quickly.
Since the bug fix for the compression ratio is going to change existing behavior
(though it will be doing the right thing now!), it might need some discussion.

http://codereview.appspot.com/2319044/diff/1/java/gadgets/src/test/java/org/a...
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java
(right):

http://codereview.appspot.com/2319044/diff/1/java/gadgets/src/test/java/org/a...
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java:34:
public void testEfficientGIF() throws Exception {
Should this test name change?
Also, I think you should ask Paul Lindner or someone as to how we can create and
add a gif that can be used for the original test (testEfficientGif).

[satya3656, 2010-10-08 08:57:38]

Keeping only bug related changes in this change

[satya3656, 2010-10-11 05:20:09]

sync to head (r1021239)

[anupama.dutta, 2010-10-11 06:19:51]

LGTM. Please send out dev@ for further review adding Paul and Gagan explicitly.

http://codereview.appspot.com/2319044/diff/13001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java
(right):

http://codereview.appspot.com/2319044/diff/13001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java:51:
* Defaults
Defaults -> Defaults for usage in tests.

[satya3656, 2010-10-11 07:32:48]

http://codereview.appspot.com/2319044/diff/13001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java
(right):

http://codereview.appspot.com/2319044/diff/13001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java:51:
* Defaults
On 2010/10/11 06:19:52, anupama.dutta wrote:
> Defaults -> Defaults for usage in tests.

Done.

[satya3656, 2010-10-11 07:33:29]

Addressing the comments.

[gagan.goku, 2010-10-12 09:54:38]

Change looks good, but 1 question:
Where do the tests pick their compression value from: the value set in
shindig.properties, or the default value set in OptimizerConfig() constructor ?

Also, maybe you should add an image with compression quality as say 70% to make
sure that we really fixed the bug.

Thanks for figuring out and fixing the awesome bug :)

[satya3656, 2010-10-12 14:31:18]

On 2010/10/12 09:54:38, gagan.goku wrote:
> Change looks good, but 1 question:
> Where do the tests pick their compression value from: the value set in
> shindig.properties, or the default value set in OptimizerConfig() constructor
?
default values set from OptimizerConfig().
 
> Also, maybe you should add an image with compression quality as say 70% to
make
> sure that we really fixed the bug.
Added

> Thanks for figuring out and fixing the awesome bug :)

[satya3656, 2010-10-12 14:35:38]

Addressing the comments.

[satya3656, 2010-10-12 14:36:58]

Addressing the comments.

[gagan.goku, 2010-10-12 16:39:43]

Lgtm. This is a funny bug fix :)

[plindner1, 2010-10-12 16:43:23]

lgtm

http://codereview.appspot.com/2319044/diff/28001/java/gadgets/src/main/java/o...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java
(right):

http://codereview.appspot.com/2319044/diff/28001/java/gadgets/src/main/java/o...
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/OptimizerConfig.java:51:
* Defaults for usuage in tests.
nit - s/usuage/usage/

[gagan.goku, 2010-10-14 04:32:58]

Build looks nice. Committed as r1022374