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

fix precedence for where the projectId is acquired from#193

Merged
ofrobots merged 4 commits intogoogleapis:masterfrom
ofrobots:project-id-precedence
Dec 16, 2016
Merged

fix precedence for where the projectId is acquired from#193
ofrobots merged 4 commits intogoogleapis:masterfrom
ofrobots:project-id-precedence

Conversation

@ofrobots
Copy link
Copy Markdown
Contributor

@ofrobots ofrobots commented Dec 15, 2016

No description provided.

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

ofrobots commented Dec 15, 2016

apologies for some of the formatting/indentation changes.

ofrobots
ofrobots commented Dec 15, 2016
View reviewed changes
src/controller.js
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 Outdated
// 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
Copy Markdown
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 loggerModule = require('@google/cloud-diagnostics-common').logger;
var defaultConfig = require('../../src/config.js').debug;
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 Outdated
};
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 Outdated
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 Outdated

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

2 more reviewers

@cristiancavalli cristiancavalli cristiancavalli left review comments

@matthewloring matthewloring matthewloring approved these changes

Reviewers whose approvals may not affect merge requirements

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