Removed dependency on @google/cloud-diagnostics-common#215
Removed dependency on @google/cloud-diagnostics-common#215kjin merged 4 commits intogoogleapis:masterfrom
Conversation
googlebot
added
the
cla: yes
cristiancavalli
left a comment
cristiancavalli
left a comment
There was a problem hiding this comment.
This LGTM with a couple questions
| return nock('http://metadata.google.internal') | ||
| .get('/computeMetadata/v1/project/numeric-project-id') | ||
| .get('/computeMetadata/v1/project/project-id') | ||
| .reply(reply); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
|
||
| debuglet.once('registered', function(id) { | ||
| assert.equal(id, DEBUGGEE_ID); | ||
| assert.equal(debuglet.debuggee_.project, projectId); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| }); | ||
|
|
||
| it('should use GCLOUD_PROJECT in lieu of config.projectId', function(done) { | ||
| process.env.GCLOUD_PROJECT = '11020304f2934-b'; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/debug-logger.js
Outdated
| newLogger[logLevel].interval = function(msg, interval) { | ||
| return newLogger[logLevel](formatInterval(msg, interval)); | ||
| }; | ||
| }); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@ofrobots PTAL |
src/debug-logger.js
Outdated
| @@ -0,0 +1,71 @@ | |||
| /** | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
system-test/test-e2e.js
Outdated
| var Debugger = require('../test/debugger.js'); | ||
|
|
||
| var CLUSTER_WORKERS = 3; | ||
| var CLUSTER_WORKERS = 1; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
ofrobots
left a comment
ofrobots
left a comment
There was a problem hiding this comment.
LGTM.
It would have been better to do the numericProjectId -> projectId change as a follow-on so that this PR could be focussed primarily on the package.json change. That makes the reviewing easier.
|
@ofrobots Sounds good, I'll try to keep future PRs more singular in purpose. |
4 participants
Removed all references to
@google/cloud-diagnostics-common, relying on equivalent functionality in@google-cloud/common.To facilitate this a new augmented
debug-loggerclass is introduced. I'm proposing to handle breakpoint and interval logging slightly differently than before, PTAL.1/9: Also removed references to metadata endpoint
numeric-project-id, and added tests to check thatGCLOUD_PROJECTandconfig.projectIdare both respected. May be helpful to look at the two commits separately.