You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/10/25 17:23:43 UTC
[jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Private injection requires reflection and limits DI choices. Binders don't need to check null as jclouds already checks null for any param not marked nullable.
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds-labs-google adrian.cleanup-binders
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/64
-- Commit Summary --
* Cleanup binders in GCE: don't use sneaky injection. don't redundantly check null.
-- File Changes --
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/DiskCreationBinder.java (23)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/FirewallBinder.java (33)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/GoogleComputeEngineErrorHandler.java (5)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/InstanceBinder.java (32)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/MetadataBinder.java (35)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/RouteBinder.java (33)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineHttpApiModule.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/FirewallApi.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ProjectApi.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/RouteApi.java (2)
R google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/binders/DiskCreationBinderTest.java (2)
R google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/binders/GoogleComputeEngineErrorHandlerTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/64.patch
https://github.com/jclouds/jclouds-labs-google/pull/64.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Adrian Cole <no...@github.com>.
> @@ -14,12 +14,10 @@
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
> -package org.jclouds.googlecomputeengine.handlers;
> +package org.jclouds.googlecomputeengine.binders;
nope. good catch! will fix.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64/files#r19376685
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Andrea Turli <no...@github.com>.
>
> public class DiskCreationBinder extends BindToJsonPayload {
>
> - @Inject
> - public DiskCreationBinder(Json jsonBinder) {
> + @Inject public DiskCreationBinder(Json jsonBinder) {
Is this style preferable? why?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64/files#r19376664
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #176](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/176/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60486303
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1547](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1547/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60491321
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1540](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1540/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60486382
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #182](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/182/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60491015
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Adrian Cole <no...@github.com>.
merged into master. 1.8.x is a mess. hopefully we can pull this out of labs for 1.8.2
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60492581
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Andrea Turli <no...@github.com>.
> - */
> - @Override
> - public <R extends HttpRequest> R bindToRequest(R request, Map<String, Object> postParams) {
> - FirewallOptions options = (FirewallOptions) checkNotNull(postParams.get("options"), "firewallOptions");
> - String name = (String) checkNotNull(postParams.get("name"), "name");
> - URI network = (URI) checkNotNull(postParams.get("network"), "network");
> - options.name(name);
> - options.network(network);
> + private final BindToJsonPayload jsonBinder;
> +
> + @Inject FirewallBinder(BindToJsonPayload jsonBinder){
> + this.jsonBinder = jsonBinder;
> + }
> +
> + @Override public <R extends HttpRequest> R bindToRequest(R request, Map<String, Object> postParams) {
> + FirewallOptions options = (FirewallOptions) postParams.get("options");
didn't know that the request was already not-null checked. nice!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64/files#r19376667
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Andrea Turli <no...@github.com>.
lgtm, very minor comments
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64#issuecomment-60490501
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Adrian Cole <no...@github.com>.
>
> public class DiskCreationBinder extends BindToJsonPayload {
>
> - @Inject
> - public DiskCreationBinder(Json jsonBinder) {
> + @Inject public DiskCreationBinder(Json jsonBinder) {
less lines, explicitly states that this is an injected constructor. This is
a style I've seen common in recent google and square projects, since they
invented Inject, and the former style looks uglier, I prefer to follow.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64/files#r19376684
Re: [jclouds-labs-google] Cleanup binders in GCE: don't use sneaky
injection. don't redundantly check null (#64)
Posted by Andrea Turli <no...@github.com>.
> @@ -14,12 +14,10 @@
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
> -package org.jclouds.googlecomputeengine.handlers;
> +package org.jclouds.googlecomputeengine.binders;
does this need to be moved to binders?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/64/files#r19376673