Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor public api to match plugin api #393

Merged
merged 1 commit into from
Feb 22, 2017
Merged

Refactor public api to match plugin api #393

merged 1 commit into from
Feb 22, 2017

Conversation

matthewloring
Copy link
Contributor

@matthewloring matthewloring commented Feb 18, 2017 •
edited
Loading

Fixes #330

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 18, 2017
@matthewloring
Copy link
Contributor Author

PTAL.

@matthewloring matthewloring added this to the Beta milestone Feb 20, 2017
kjin
kjin approved these changes Feb 21, 2017
View reviewed changes
Copy link
Contributor

@kjin kjin left a comment

Choose a reason for hiding this comment

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

LGTM w/ small comments.

src/trace-plugin-interface.js Outdated
* Creates and returns a new ChildSpan object nested within the root span object
* returned by getRootSpan. If there is no current RootSpan object, this
* function returns null.
* Creates and returns a new ChildSpan object nested within the root span.If

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/trace-plugin-interface.js Outdated
* @param {object} options An object that specifies options for how the child
* span is created and propogated.
* @returns A new ChildSpan object, or null if there is no active root span.
*/
PluginAPI.prototype.createChildSpan = function(options) {
var rootSpan = this.getRootSpan();
var rootSpan = this.getRootSpan_();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

index.js
},
setTransactionName: function() {},
addTransactionLabel: function() {}
enhancedDatabaseReportingEnabled: function() { return false; },

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

ofrobots
ofrobots approved these changes Feb 21, 2017
View reviewed changes
Copy link
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

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

LGTM w/ nit/question.

index.js
wrap: function(fn) { return fn; },
wrapEmitter: function(ee) { return ee; },
constants: constants,
labels: TraceLabels
};

/** @private */

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@DominicKramer
Copy link
Contributor

LGTM

@matthewloring matthewloring merged commit f641802 into googleapis:master Feb 22, 2017
@matthewloring matthewloring deleted the public-api branch February 22, 2017 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ofrobots ofrobots ofrobots approved these changes

@kjin kjin kjin approved these changes

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.
5 participants
@matthewloring @DominicKramer @ofrobots @kjin @googlebot