Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
>BTW @nacx I saw the comments on the site PR regarding the slack channel. Would >it be possible to invite @btrishkin and I? Sure. Send mean email with your emails and I'll send the invites. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348238122
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
andreaturli requested changes on this pull request.
thanks for the great start @btrishkin
Some initial thoughts on you PR, could you please have a look at them?
> +import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Set;
+
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+
+@Test(groups = "unit", testName = "DatacenterToLocationTest")
+public class DatacenterToLocationTest {
In jclouds we test those kind of functions already using a slightly different
pattern - see
https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/functions/RegionToLocationTest.java
for a great example
> +
+ parentLocations = ImmutableSet.of(new
LocationBuilder().scope(LocationScope.PROVIDER).id("parentId")
+.iso3166Codes(new ArrayList()).metadata(new
HashMap()).description("parentDescription").build());
+
+ datacenter = Datacenter.builder().displayName("US - West").city("Santa
Clara").state("California").country("US")
+
.vpnUrl("https://na3.cloud-vpn.net";).ftpsHost("ftps-na.cloud-vpn.net").networking(Networking.builder()
+
.properties(Collections.singletonList(Property.create("MAX_SERVER_TO_VIP_CONNECTIONS",
"20")))
+
.type("1").maintenanceStatus("NORMAL").build()).hypervisor(hypervisor).backup(
+
Backup.builder().maintenanceStatus("NORMAL").type("COMMVAULT")
+
.properties(Collections.emptyList()).build()).consoleAccess(
+
ConsoleAccess.builder().properties(Collections.emptyList()).maintenanceStatus("NORMAL")
+.build()).monitoring(
+
Monitoring.builder().maintenanceStatus("NORMAL").properties(Collections.emptyList())
+.build()).type("MCP 1.0").id("NA3").build();
+
+ justProvider = createNiceMock(JustProvider.class);
do you need to mock the `justProvider` or you can maybe re-use the pattern used
at
https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/functions/RegionToLocationTest.java#L41-L42
?
> +import org.jclouds.dimensiondata.cloudcontrol.domain.Server;
+import org.jclouds.dimensiondata.cloudcontrol.domain.State;
+import
org.jclouds.dimensiondata.cloudcontrol.domain.internal.ServerWithExternalIp;
+import org.jclouds.dimensiondata.cloudcontrol.features.NetworkApi;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.Date;
+
+import static org.easymock.EasyMock.expect;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+
+@Test(groups = "unit", testName = "DatacenterToLocationTest")
I think it is ` testName = ServerToServerWithExternalIpTest`
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/423#pullrequestreview-80234663
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
Sorry @trevorflanagan and @btrishkin for the confusion around ICLA -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348237647
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
@btrishkin I'm not sure I understand the title of this PR: you mention https://issues.apache.org/jira/browse/JCLOUDS-72 and https://issues.apache.org/jira/browse/JCLOUDS-75 but they are not related to dimensiondata, are they? Please to simplify the review, could you open a jira issue and rename the PR accordingly please? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348237467
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
BTW @nacx I saw the comments on the site PR regarding the slack channel. Would it be possible to invite @btrishkin and I? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348237362
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
thanks @btrishkin for your contribution. First of all, welcome to the jclouds community! I may have missed that, but did you sign already the ICLA? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348236023
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
I think it was a small confusion. ICLAs are not required for Apache contributors, only for committers. Contributors must just assume and accept (and sending a patch or opening a PR means that, that's why you copy our license headers too) that their contributions are meant to be distributed under the terms of the Apache License. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348235632
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
hey @andreaturli we were not aware of the requirement to sign an ICLA. We will have to check internally to see if Dimension Data have a process in place already for this. BTW I have not signed one either. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348233396
Re: [jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
thanks @btrishkin for your contribution. First of all, welcome to the jclouds community! I may have missed that, but did you sign already the ICLA? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423#issuecomment-348229954
[jclouds/jclouds-labs] JCLOUD-72 / JCLOUD-75 - DatacenterToLocation / ServerToServerWithExternalIp Functions (#423)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/423 -- Commit Summary -- * JCLOUD-72 / JCLOUD-75 - Implement DatacenterToLocation / ServerToServerWithExternalIp Functions -- File Changes -- A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/compute/function/DatacenterToLocation.java (52) A dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/compute/function/ServerToServerWithExternalIp.java (60) A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/compute/function/DatacenterToLocationTest.java (95) A dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/compute/function/ServerToServerWithExternalIpTest.java (118) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/423.patch https://github.com/jclouds/jclouds-labs/pull/423.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/423
