jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ignasi Barrera <notificati...@github.com>
Subject Re: [jclouds/jclouds-labs] [JCLOUDS-1219] initial commit for Packet.net API (#337)
Date Tue, 03 Jan 2017 15:27:21 GMT
nacx requested changes on this pull request.

Thanks @andreaturli!

Can't wait to have this provider :)

Just some minor comments and general considerations about using primitives where we have Numbers
and Boolean variables that are non-nullable.

> + * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package domain;

Change packages to `org.jclouds.packet`

> +            checkArgument(state.isPresent(), "Expected one of %s but was %s", Joiner.on(',').join(State.values()),
value);
+            return state.get();
+        }
+    }
+
+    public abstract String id();
+    public abstract String shortId();
+    public abstract String hostname();
+    @Nullable
+    public abstract String description();
+    public abstract State state();
+    public abstract List<String> tags();
+    public abstract String billingCycle();
+    public abstract String user();
+    public abstract String iqn();
+    public abstract Boolean locked();

Can this be a primitive type? Is this ever null?

> +
+@AutoValue
+public abstract class IpAddress {
+
+    public abstract String id();
+    public abstract Integer addressFamily();
+    public abstract String netmask();
+    public abstract Boolean publicAddress();
+    public abstract Integer cidr();
+    public abstract Boolean management();
+    public abstract Boolean manageable();
+    public abstract Href assignedTo();
+    public abstract String network();
+    public abstract String address();
+    public abstract String gateway();
+    public abstract String href();

Are all the integer and boolean fields here nullable? Can we change them to primitives?

> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package domain;
+
+import java.util.Set;
+
+import org.jclouds.json.SerializedNames;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+
+/**
+ * {"id":"06e21644-a769-11e6-80f5-76304dec7eb7","slug":"alpine_3","name":"Alpine 3","distro":"alpine","version":"3","provisionable_on":[]}
+ */

This json an easily become outdated as the API evolves. Remove it from the javadoc? (also
apply to the other classes).

> +import java.security.PublicKey;
+
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import com.google.auto.value.AutoValue;
+
+@AutoValue
+public abstract class Project {
+
+    public abstract String id();
+    public abstract String name();
+    @Nullable
+    public abstract String fingerprint();
+    @Nullable
+    public abstract PublicKey publicKey();

You will probably need a custom adapter to be able to serialize/deserialize a PublicKey object.
Consider adding it to a parser module like in the [DigitalOcean2 provider](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/config/DigitalOceanParserModule.java#L66).

-- 
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/337#pullrequestreview-14947791
Mime
View raw message