michaelwiles / google-gin

Automatically exported from code.google.com/p/google-gin
Apache License 2.0
0 stars 0 forks source link

Select gin modules for Ginjector from property value #142

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In issue #129, we introduced the ability to specify gin modules in a GWT module 
property value. These specified gin modules get applied to all ginjectors that 
are created in an application. Instead we should think about letting the 
ginjector (optionally?) determine what the property value should be named from 
which it accepts modules.

Original issue reported on code.google.com by aragos on 22 Feb 2011 at 9:49

GoogleCodeExporter commented 9 years ago
As mentioned in issue #129 there are two options here:
a) Specify one or more configuration properties to use from the @GinModules 
annotation (or a similar annotation)
b) Use the ginjector class name to generate a string that uniquely identifies a 
multi-valued configuration property for that ginjector

My personal preference is (a) because it could be extended to support not only 
configuration properties, but deferred binding properties that can be 
controlled conditionally, making it possible to switch a module based on, say, 
the browser type.

We could keep backward compatibility with #129 but I suggest we scrap the old 
global multi-valued configuration property in favor or either (a) or (b).

I'm happy to take the lead on this one.

Original comment by philippe.beaudoin on 22 Feb 2011 at 10:39

GoogleCodeExporter commented 9 years ago
I also prefer (a), go ahead and do it! :)

Original comment by aragos on 22 Feb 2011 at 11:12

GoogleCodeExporter commented 9 years ago
Good. Just to get me started, would you rather we add a field to @GinModules or 
we define a new annotation @GinModuleProperties?

Original comment by philippe.beaudoin on 22 Feb 2011 at 11:19

GoogleCodeExporter commented 9 years ago
Can you see any functional difference? If there is not, I'd probably prefer to 
keep it in the GinModules annotation.

Original comment by aragos on 23 Feb 2011 at 4:45

GoogleCodeExporter commented 9 years ago
No functional difference, syntactic preference only. I'll go with @GinModules.

Original comment by philippe.beaudoin on 23 Feb 2011 at 4:50

GoogleCodeExporter commented 9 years ago
After some investigation, it will not be possible to define modules as deferred 
binding properties. The reason is that GWT refuses to parse a deferred binding 
property that contains "." in the value. That is, the following fails when 
parsing XML:

  <!-- Selection property used to specify gin modules -->
  <!-- Selection property used to specify gin modules -->
  <define-property name="agent.dependent.ginmodule" values="com.google.gwt.inject.client.configurationmodules.ModuleAllBrowsers,com.google.gwt.inject.client.configurationmodules.ModuleIe6"/>
  <set-property name="agent.dependent.ginmodule" value="com.google.gwt.inject.client.configurationmodules.ModuleAllBrowsers" />
  <set-property name="agent.dependent.ginmodule" value="com.google.gwt.inject.client.configurationmodules.ModuleIe6" >
    <when-property-is name="user.agent" value="ie6" />
  </set-property>

Original comment by philippe.beaudoin on 2 Mar 2011 at 7:08

GoogleCodeExporter commented 9 years ago
Patch is up for review:
http://codereview.appspot.com/4258046/

Original comment by philippe.beaudoin on 2 Mar 2011 at 7:54

GoogleCodeExporter commented 9 years ago
Just quick ping on this... :)

Original comment by philippe.beaudoin on 16 Mar 2011 at 4:30

GoogleCodeExporter commented 9 years ago
I made some comments on your code review, please address those first. :)

Original comment by aragos on 17 Mar 2011 at 9:25

GoogleCodeExporter commented 9 years ago

Original comment by aragos on 17 Mar 2011 at 9:25

GoogleCodeExporter commented 9 years ago
Committed in r187.

Original comment by aragos on 18 Mar 2011 at 11:50