Closed GoogleCodeExporter closed 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
I also prefer (a), go ahead and do it! :)
Original comment by aragos
on 22 Feb 2011 at 11:12
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
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
No functional difference, syntactic preference only. I'll go with @GinModules.
Original comment by philippe.beaudoin
on 23 Feb 2011 at 4:50
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
Patch is up for review:
http://codereview.appspot.com/4258046/
Original comment by philippe.beaudoin
on 2 Mar 2011 at 7:54
Just quick ping on this... :)
Original comment by philippe.beaudoin
on 16 Mar 2011 at 4:30
I made some comments on your code review, please address those first. :)
Original comment by aragos
on 17 Mar 2011 at 9:25
Original comment by aragos
on 17 Mar 2011 at 9:25
Committed in r187.
Original comment by aragos
on 18 Mar 2011 at 11:50
Original issue reported on code.google.com by
aragos
on 22 Feb 2011 at 9:49