compute: implement subnetworks #1435

ashleyschuett · 2016-07-15T13:04:25Z

googlebot · 2016-07-15T13:04:26Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

lib/compute/network.js

@@ -215,6 +221,135 @@ Network.prototype.createFirewall = function(name, config, callback) {
 };

 /**
+ * Create a subnetwork in this subnetwork.


jgeewax · 2016-07-15T13:19:21Z

Awesome -- thanks! Can you take a look at the CLA ? We can't merge without that :(

ashleyschuett · 2016-07-15T13:22:36Z

I updated it to be for the email address I used in the commits, but it doesn't seem to be updating.

googlebot · 2016-07-15T13:22:38Z

CLAs look good, thanks!

stephenplusplus · 2016-07-15T16:21:17Z

Thank you for helping! :)

The CI check stopped early because of the linter issues (https://travis-ci.org/GoogleCloudPlatform/gcloud-node/jobs/145010438#L917) -- if you push the changes that fix them, the rest of the tests should execute.

Were you interested in writing the unit tests for this change? If not, I'll create a PR branched off of this one with the changes.

coveralls · 2016-07-15T18:19:03Z

Coverage decreased (-1.1%) to 98.888% when pulling f992360 on containership:master into d995fb6 on GoogleCloudPlatform:master.

stephenplusplus · 2016-07-15T19:05:30Z

Looks like another bug that our linter didn't catch: https://travis-ci.org/GoogleCloudPlatform/gcloud-node/jobs/145071576#L534

lib/compute/network.js:268
  let region = new Region(this.compute, config.region);
  ^^^
SyntaxError: Unexpected strict mode reserved word

We still have to support 0.12 :\

coveralls · 2016-07-15T19:22:36Z

Coverage decreased (-1.1%) to 98.888% when pulling 8e9166b on containership:master into d995fb6 on GoogleCloudPlatform:master.

ashleyschuett · 2016-07-15T19:24:28Z

what other tests need to be written yet? I'm getting the same coverage results after writing the system tests.

system-test/compute.js

+    };
+
+
+    let SUBNETWORK_CONFIG = {


stephenplusplus · 2016-07-15T19:30:05Z

Every line of code in the lib directory gets tested in the matching folder in the test directory. In this case, each of the methods you created require testing. Copying and pasting tests should go pretty far, but if this is something you'd rather leave for us, just let me know.

coveralls · 2016-07-15T19:45:11Z

Coverage decreased (-1.1%) to 98.888% when pulling 90248b8 on containership:master into d2780ca on GoogleCloudPlatform:master.

coveralls · 2016-07-15T22:27:57Z

Coverage remained the same at 100.0% when pulling c064a32 on containership:master into d2780ca on GoogleCloudPlatform:master.

ashleyschuett · 2016-07-19T13:01:00Z

Let me know if there is anything missing, or I need to do anything else!

lib/compute/index.js

+ * }, callback);
+ *
+ * //-
+ * // Get the firewalls from your project as a readable object stream.


lib/compute/subnetwork.js

+     * @param {function=} callback - The callback function.
+     * @param {?error} callback.err - An error returned while making this
+     *     request.
+     * @param {object} callback.metadata - The address's metadata.


stephenplusplus · 2016-07-19T15:01:17Z

Thanks, it looks pretty great! I'm done with the review for now. Let me know if you have any thoughts on the feedback I left. I'll look at the tests after any updates you might make.

ashleyschuett · 2016-07-19T20:04:51Z

I'd defiantly like to take you up on getting some help with the test's. I'm not sure what I'm missing and am not sure how to test some of the functionality properly.

stephenplusplus · 2016-07-19T20:37:57Z

No problem, I'm going over them now.

stephenplusplus · 2016-07-20T12:51:34Z

PR sent :) https://github.com/containership/gcloud-node/pull/1

tests refactor

googlebot · 2016-07-20T13:19:58Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

coveralls · 2016-07-20T13:23:19Z

Coverage decreased (-0.05%) to 99.946% when pulling 3d6ff28 on containership:master into d2780ca on GoogleCloudPlatform:master.

stephenplusplus · 2016-07-20T13:24:34Z

Oh geez, I made the coverage worse. That's sad. I'll be right on that!

fix coverage

coveralls · 2016-07-20T13:53:51Z

Coverage remained the same at 100.0% when pulling 4a1921c on containership:master into d2780ca on GoogleCloudPlatform:master.

stephenplusplus · 2016-07-20T14:06:11Z

Yay! Thank you very much!

ashleyschuett · 2016-07-20T14:19:31Z

Awesome! Thanks for all the help! Do I need to do anything for the cia thing, or is that something you need to accept?

stephenplusplus · 2016-07-20T14:21:40Z

Nope, it's all set. We just confused it when I PRd to your fork. Since
you've already signed, we're all good!
On Wed, Jul 20, 2016 at 10:19 AM Ashley Schuett notifications@github.com
wrote:

Awesome! Thanks for all the help! Do I need to do anything for the cia
thing, or is that something you need to accept?

—
You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
#1435 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAsIaBqa77VhdtTllV4oAC4lVZclV06Dks5qXi51gaJpZM4JNYLb
.

googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Jul 15, 2016

jgeewax reviewed Jul 15, 2016
View reviewed changes

lib/compute/network.js

@@ -215,6 +221,135 @@ Network.prototype.createFirewall = function(name, config, callback) {

};

/**

* Create a subnetwork in this subnetwork.

This comment was marked as spam.

Sign in to view

googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jul 15, 2016

stephenplusplus added the api: compute Issues related to the Compute Engine API. label Jul 15, 2016

stephenplusplus reviewed Jul 15, 2016
View reviewed changes

system-test/compute.js

};

let SUBNETWORK_CONFIG = {

This comment was marked as spam.

Sign in to view

authtestprod added 2 commits July 15, 2016 18:12

compute: implement subnetworks

01d9442

adding unit tests

c064a32

stephenplusplus reviewed Jul 19, 2016
View reviewed changes

lib/compute/index.js

* }, callback);

*

* //-

* // Get the firewalls from your project as a readable object stream.

This comment was marked as spam.

Sign in to view

stephenplusplus reviewed Jul 19, 2016
View reviewed changes

lib/compute/subnetwork.js

* @param {function=} callback - The callback function.

* @param {?error} callback.err - An error returned while making this

* request.

* @param {object} callback.metadata - The address's metadata.

This comment was marked as spam.

Sign in to view

made changes

df4ac67

Stephen Sawchuk and others added 2 commits July 20, 2016 09:17

tests refactor

54de7d6

Merge pull request #1 from stephenplusplus/containership--subnetworks

3d6ff28

tests refactor

googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jul 20, 2016

Stephen Sawchuk and others added 2 commits July 20, 2016 09:44

fix coverage

61c207a

Merge pull request #2 from stephenplusplus/containership--subnetworks

4a1921c

fix coverage

stephenplusplus merged commit 5689f96 into googleapis:master Jul 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compute: implement subnetworks #1435

compute: implement subnetworks #1435

ashleyschuett commented Jul 15, 2016

googlebot commented Jul 15, 2016

This comment was marked as spam.

jgeewax commented Jul 15, 2016

ashleyschuett commented Jul 15, 2016

googlebot commented Jul 15, 2016

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 •

edited

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 •

edited

ashleyschuett commented Jul 15, 2016

This comment was marked as spam.

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 •

edited

coveralls commented Jul 15, 2016 •

edited

ashleyschuett commented Jul 19, 2016

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Jul 19, 2016

ashleyschuett commented Jul 19, 2016

stephenplusplus commented Jul 19, 2016

stephenplusplus commented Jul 20, 2016

googlebot commented Jul 20, 2016

coveralls commented Jul 20, 2016 •

edited

stephenplusplus commented Jul 20, 2016

coveralls commented Jul 20, 2016 •

edited

stephenplusplus commented Jul 20, 2016

ashleyschuett commented Jul 20, 2016

stephenplusplus commented Jul 20, 2016

compute: implement subnetworks #1435

compute: implement subnetworks #1435

Conversation

ashleyschuett commented Jul 15, 2016

googlebot commented Jul 15, 2016

This comment was marked as spam.

jgeewax commented Jul 15, 2016

ashleyschuett commented Jul 15, 2016

googlebot commented Jul 15, 2016

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 • edited

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 • edited

ashleyschuett commented Jul 15, 2016

This comment was marked as spam.

stephenplusplus commented Jul 15, 2016

coveralls commented Jul 15, 2016 • edited

coveralls commented Jul 15, 2016 • edited

ashleyschuett commented Jul 19, 2016

This comment was marked as spam.

This comment was marked as spam.

stephenplusplus commented Jul 19, 2016

ashleyschuett commented Jul 19, 2016

stephenplusplus commented Jul 19, 2016

stephenplusplus commented Jul 20, 2016

googlebot commented Jul 20, 2016

coveralls commented Jul 20, 2016 • edited

stephenplusplus commented Jul 20, 2016

coveralls commented Jul 20, 2016 • edited

stephenplusplus commented Jul 20, 2016

ashleyschuett commented Jul 20, 2016

stephenplusplus commented Jul 20, 2016

coveralls commented Jul 15, 2016 •

edited

coveralls commented Jul 15, 2016 •

edited

coveralls commented Jul 15, 2016 •

edited

coveralls commented Jul 15, 2016 •

edited

coveralls commented Jul 20, 2016 •

edited

coveralls commented Jul 20, 2016 •

edited