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

Change v8debugAPI clear to async.#327

Merged
gaofanmichael merged 2 commits intogoogleapis:masterfrom
gaofanmichael:feature/async
Sep 11, 2017
Merged

Change v8debugAPI clear to async.#327
gaofanmichael merged 2 commits intogoogleapis:masterfrom
gaofanmichael:feature/async

Conversation

@gaofanmichael
Copy link
Copy Markdown
Contributor

@gaofanmichael gaofanmichael commented Sep 7, 2017

This PR aims to change the exisiting v8debug API clear function to async
in order to prepare for the migration debug api from v8 to inspector,
as inspector api is async.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 7, 2017
ofrobots
ofrobots approved these changes Sep 7, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@ofrobots ofrobots left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM w/ nit.

EDIT: once the CI is happy.

src/agent/v8debugapi.ts Outdated
'A script matching the source file was not found loaded on the debuggee',
SOURCE_FILE_AMBIGUOUS: 'Multiple files match the path specified',
V8_BREAKPOINT_ERROR: 'Unable to set breakpoint in v8',
V8_BREAKPOINT_CLAER_ERROR: 'Unable to clear breakpoint in v8',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

DominicKramer
DominicKramer reviewed Sep 7, 2017
View reviewed changes
src/agent/debuglet.ts Outdated
delete this.activeBreakpointMap_[breakpoint.id as string];
if (this.v8debug_) {
this.v8debug_.clear(breakpoint);
this.v8debug_.clear(breakpoint, function(err) {

This comment was marked as spam.

Sign in to view

test/test-v8debugapi.ts
assert.ifError(err);
api.clear(bp);
done();
api.clear(bp, function(err) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

test/test-v8debugapi.ts
assert.ifError(err);
api.clear(bp);
done();
api.clear(bp, function(err) {

This comment was marked as spam.

Sign in to view

test/test-v8debugapi.ts Outdated
'Locals and arguments are only displayed.*config.capture.maxExpandFrames=0'
));
api.clear(bp);
config.capture.maxExpandFrames = oldCount;

This comment was marked as spam.

Sign in to view

test/test-v8debugapi.ts Outdated
assert.equal(topFrame.locals[0].name, 'n');
assert.equal(topFrame.locals[0].value, '2');
api.clear(bp);
config.capture.maxFrames = oldMax;

This comment was marked as spam.

Sign in to view

test/test-v8debugapi.ts Outdated
assert(((fooVal as any).status as any).isError);

api.clear(bp);
config.capture.maxDataSize = oldMaxData;

This comment was marked as spam.

Sign in to view

src/agent/state.ts
// tslint:disable-next-line:variable-name
const ScopeType = vm.runInDebugContext('ScopeType');
const assert = debugAssert(process.env.CLOUD_DEBUG_ASSERTIONS);
const assert = debugAssert(!!process.env.CLOUD_DEBUG_ASSERTIONS);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/agent/v8debugapi.ts Outdated
'A script matching the source file was not found loaded on the debuggee',
SOURCE_FILE_AMBIGUOUS: 'Multiple files match the path specified',
V8_BREAKPOINT_ERROR: 'Unable to set breakpoint in v8',
V8_BREAKPOINT_CLAER_ERROR: 'Unable to clear breakpoint in v8',

This comment was marked as spam.

Sign in to view

test/test-debuglet.ts Outdated

describe('setup', function() {
before(function() { oldGP = process.env.GCLOUD_PROJECT; });
before(function() { oldGP = String(process.env.GCLOUD_PROJECT); });

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

test/test-debuglet.ts
it('should respect GCLOUD_DEBUG_LOGLEVEL', function(done) {
process.env.GCLOUD_PROJECT = '11020304f2934';
process.env.GCLOUD_DEBUG_LOGLEVEL = 3;
process.env.GCLOUD_DEBUG_LOGLEVEL = '3';

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

matthewloring
matthewloring reviewed Sep 8, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@matthewloring matthewloring left a comment

Choose a reason for hiding this comment

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

No need to change this PR but in general we do not include punctuation in commit titles.

src/agent/v8debugapi.ts
v8.setListener(null);
}
return true;
return setImmediate(function() {

This comment was marked as spam.

Sign in to view

test/test-debuglet.ts Outdated

describe('setup', function() {
before(function() { oldGP = process.env.GCLOUD_PROJECT; });
before(function() { oldGP = String(process.env.GCLOUD_PROJECT); });

This comment was marked as spam.

Sign in to view

@gaofanmichael
Change v8debugAPI clear to async.
f1425d6 
This PR aims to change the exisiting v8debug API clear function to async
in order to prepare for the migration debug api from v8 to inspector,
as inspector api is async.
@gaofanmichael
resolve comments
38568c1
@gaofanmichael gaofanmichael merged commit 9ecff8b into googleapis:master Sep 11, 2017
@gaofanmichael gaofanmichael deleted the feature/async branch September 11, 2017 17:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kjin kjin Awaiting requested review from kjin

3 more reviewers

@matthewloring matthewloring matthewloring left review comments

@DominicKramer DominicKramer DominicKramer left review comments

@ofrobots ofrobots ofrobots 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@gaofanmichael @ofrobots @matthewloring @kjin @DominicKramer @jmdobry @googlebot