search

sgkhode / google-gin

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

small cleanups as per jesse's review of r73 #23

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
From review:

[google-gin] limpbizkit commented on revision r73.
Details are at http://code.google.com/p/google-gin/source/detail?r=73

Score: Positive

Line-by-line comments:

File: /trunk/src/com/google/gwt/inject/rebind/BindingsProcessor.java (r73)
===============================================================================

Line 420:     if (binding == null) {
-------------------------------------------------------------------------------
really necesssary?

File: /trunk/src/com/google/gwt/inject/rebind/LieToGuiceModule.java (r73)
===============================================================================

Line 34:   private final List<Module> implicitBindings = new
ArrayList<Module>();
-------------------------------------------------------------------------------
kinda awkward variable name, a List&lt;Module&gt; being called 'bindings'
I don't know about the use of the word 'implicit' . . .  could you just
call them GwtCreateBindings? I assume that's the only place they come from...

Line 61:   private class ImplicitBindingModule<T> implements Module,
Provider<T> {
-------------------------------------------------------------------------------
Neato

File:
/trunk/src/com/google/gwt/inject/rebind/binding/CallGwtDotCreateBinding.java 
(r73)
===============================================================================

Line 37:     sb.append("GWT.create(")
-------------------------------------------------------------------------------
neato codegen

File:
/trunk/src/com/google/gwt/inject/rebind/binding/RemoteServiceProxyBinding.java
(r73)
===============================================================================

Line 41:   public RemoteServiceProxyBinding(@InjectionPoint MemberCollector
memberCollector,
-------------------------------------------------------------------------------
hmm. . . . I think we're also using the name InjectionPoint possibly for
something different . . . .

Original issue reported on code.google.com by bstoler+code@google.com on 9 Jan 2009 at 6:06

GoogleCodeExporter commented 8 years ago 
Patch and code review: http://codereview.appspot.com/12042/show

Original comment by bstoler+code@google.com on 10 Jan 2009 at 1:28

GoogleCodeExporter commented 8 years ago 
Submitted in r78.

Original comment by bstoler+code@google.com on 20 Jan 2009 at 4:13

GoogleCodeExporter commented 8 years ago

Original comment by bstoler+code@google.com on 21 Feb 2009 at 9:17