Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

fix precedence for where the projectId is acquired from #193

Merged
merged 4 commits into from
Dec 16, 2016
Merged

fix precedence for where the projectId is acquired from #193

merged 4 commits into from
Dec 16, 2016

Conversation

ofrobots
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 15, 2016
@ofrobots
Copy link
Contributor Author

apologies for some of the formatting/indentation changes.

ofrobots
ofrobots commented Dec 15, 2016
View reviewed changes
src/controller.js
@@ -132,13 +135,14 @@ Controller.prototype.register_ = function(errorMessage, callback) {
uri: API + '/debuggees/register',
method: 'POST',
json: true,
body: { debuggee: debuggee }
body: {debuggee: debuggee}

This comment was marked as spam.

Sign in to view

matthewloring
matthewloring reviewed Dec 15, 2016
View reviewed changes
src/controller.js
// We should get an error if we are not on GCP.
that.onGCP = !err;

// We perfer to use the locally available projectId as that is least

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

cristiancavalli
cristiancavalli reviewed Dec 15, 2016
View reviewed changes
Copy link
Contributor

@cristiancavalli cristiancavalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial scan looks good with a couple questions

test/standalone/test-debuglet.js
var Debuglet = require('../../src/agent/debuglet.js');
var extend = require('extend');

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

test/standalone/test-debuglet.js
@@ -44,25 +43,26 @@ var errorBp = {
location: { path: 'fixtures/foo.js', line: 2 }
};
var fakeDebug = {
request: request
request: request // avoid authing.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

matthewloring
matthewloring suggested changes Dec 16, 2016
View reviewed changes
test/standalone/test-config-credentials.js
projectId: 'project-via-config',
credentials: require('../fixtures/gcloud-credentials.json')
});
var debug = require('../../')(config);

This comment was marked as spam.

Sign in to view

test/standalone/test-config-credentials.js

describe('test-config-credentials', function() {
var debuglet = null;
var envProject = process.env.GCLOUD_PROJECT;

This comment was marked as spam.

Sign in to view

@ofrobots ofrobots added the semver: major Hint for users that this is an API breaking change. label Dec 16, 2016
@ofrobots ofrobots merged commit f7de637 into googleapis:master Dec 16, 2016
@ofrobots ofrobots deleted the project-id-precedence branch December 16, 2016 00:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cristiancavalli cristiancavalli cristiancavalli left review comments

@matthewloring matthewloring matthewloring approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement. semver: major Hint for users that this is an API breaking change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ofrobots @matthewloring @cristiancavalli @googlebot