andreaturli commented on this pull request.

thanks @ygy -- looks really useful

some comments from my quick review, haven't executed the live tests though

> +    * Windows configuration parameters
+    *
+    * @see <a 
href="https://docs.microsoft.com/en-us/rest/api/compute/virtualmachines/virtualmachines-create-or-update#bk_windowsconfig5";>docs</a>
+    */
+   public AzureTemplateOptions windowsConfiguration(WindowsConfiguration 
windowsConfiguration) {
+       this.windowsConfiguration = windowsConfiguration;
+       return this;
+    }
+
+   /**
+    * Import certificates in the Windows Certificate Store
+    *
+    * @see <a 
href="https://docs.microsoft.com/en-us/rest/api/compute/virtualmachines/virtualmachines-create-or-update#bk_srcvault";>docs</a>
+    */
+   public AzureTemplateOptions secrets(Iterable<? extends Secrets> secrets) {
+       this.secrets = (secrets == null) ? null : 
ImmutableList.copyOf(checkNotNull(secrets, "secrets"));

if all `secrets` must be non null, you may want to use the same pattern used 
for `ipOptions` above

> @@ -91,15 +90,58 @@ public static LinuxConfiguration create(final String 
> disablePasswordAuthenticati
 
       @AutoValue
       public abstract static class WinRM {
+          public enum ListenerProtocol {

is `Protocol` a better name?

> @@ -96,6 +107,11 @@ public void setup() {
       nicName = nic.name();
 
       vmName = String.format("%3.24s", System.getProperty("user.name") + RAND 
+ this.getClass().getSimpleName()).toLowerCase().substring(0, 15);
+
+      vaultResourceGroup = System.getProperty("test.vault.resource.group");

I think this makes the test harder to execute: either you add it to 
`BaseAzureComputeApiLiveTest` or the test needs to create those prelimiary 
resources during the setup and destroy during tearDown. wdyt?

-- 
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/393#pullrequestreview-39866885

Reply via email to