You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2014/08/05 14:46:57 UTC
[jclouds-labs-openstack] Adds missing constructor properties (#124)
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack missing-contructorprops
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/124
-- Commit Summary --
* Adds missing constructor properties
-- File Changes --
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/AllocationPool.java (3)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/ExternalGatewayInfo.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/ExtraDhcpOption.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/HostRoute.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/IP.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/NetworkSegment.java (2)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/RouterInterface.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/124.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/124.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Jeremy Daggett <no...@github.com>.
@zack-shoylev +1 LGTM2!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#issuecomment-51265148
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Zack Shoylev <no...@github.com>.
merged
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#issuecomment-51490264
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Zack Shoylev <no...@github.com>.
> @@ -33,6 +35,7 @@
> protected final String start;
> protected final String end;
>
> + @ConstructorProperties({"start", "end"})
I am fairly certain that if these are missing GSON will use the "unsafe" deserialization approach we want to avoid.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124/files#r15842736
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #348](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/348/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#issuecomment-51193385
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Andrew Phillips <no...@github.com>.
One minor question, but +1 - looks good to me. Thanks, @zack-shoylev!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#issuecomment-51208792
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -33,6 +35,7 @@
> protected final String start;
> protected final String end;
>
> + @ConstructorProperties({"start", "end"})
That was me with the wrong session saved :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124/files#r15844435
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Abiquo Jenkins <no...@github.com>.
> @@ -33,6 +35,7 @@
> protected final String start;
> protected final String end;
>
> + @ConstructorProperties({"start", "end"})
Previous investigations of how Gson works (in another PR) showed that, as Zack points, if the annotation is not present, the default Gson impl will be used to build the deserialized object. If there is no default constructor and the fields are final, an impl that uses sun.misc.Unsafe will be used to set the values directly in memory. We should avoid falling back to it, as that might not be available in all JVM implementations.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124/files#r15844109
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Zack Shoylev <no...@github.com>.
Closed #124.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#event-150326368
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Andrew Phillips <no...@github.com>.
> @@ -33,6 +35,7 @@
> protected final String start;
> protected final String end;
>
> + @ConstructorProperties({"start", "end"})
Just curious...are these needed? The names look the same?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124/files#r15817513
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Andrew Phillips <no...@github.com>.
> @@ -33,6 +35,7 @@
> protected final String start;
> protected final String end;
>
> + @ConstructorProperties({"start", "end"})
> I am fairly certain that if these are missing GSON will use the "unsafe" deserialization approach we want to avoid.
Ah, thanks for explaining! In that case, let's certainly keep it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124/files#r15845134
Re: [jclouds-labs-openstack] Adds missing constructor properties
(#124)
Posted by Zack Shoylev <no...@github.com>.
About to merge
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/124#issuecomment-51489464