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.

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

support groups in createBackgroundTransaction stub #175

Closed
wants to merge 1 commit into from
Closed

support groups in createBackgroundTransaction stub #175

wants to merge 1 commit into from

Conversation

nullvariable
Copy link
Contributor

The full api for createBackgroundTransaction supports three arguments with the second argument being optional. The fallback stub only supported the name and callback arguments. I've update the stub and the tests to support the optional group argument as the full version of the api does.

Thanks!

@wraithan
Copy link
Contributor

Howdy @nullvariable, sorry this got missed. The week you put this up was a very busy week for the team. I've added testing this to my todo list for this week. If you don't mind squashing your commits into a single one that'd be wonderful. Thanks!

in api.js:433 the createBackgroundTransaction function supports up to three arguments with the second argument being optional. If the api fails to load and the stub fallback is used, it will fail in cases where three arguments were used. I've updated the createBackgroundTransaction stub to support all three arguments conditionally.

tests to support group argument for createBackgroundTransaction
@nullvariable
Copy link
Contributor Author

@wraithan no worries, I think that squashed it into one commit, let me know if that didn't work.

@wraithan
Copy link
Contributor

@nullvariable So we should be merging this in this week. I was wondering how you prefer to be attributed in the release notes. We are happy to use your name as it appears on github, your github handle, or any other identifier you prefer.

@nullvariable
Copy link
Contributor Author

github handle works great, thanks!

@wraithan
Copy link
Contributor

@nullvariable Your patch was merged out of band (and rebased). Thanks for the PR!

@wraithan wraithan closed this Oct 16, 2014
cmcadams-newrelic pushed a commit to cmcadams-newrelic/node-newrelic that referenced this pull request Jan 29, 2024
…/error-fingerprinting/fast-xml-parser-and-artillery-4.1.2

chore(deps): bump fast-xml-parser and artillery in /error-fingerprinting
jsumners-nr pushed a commit to jsumners-nr/node-newrelic that referenced this pull request Apr 16, 2024
bizob2828 added a commit to bizob2828/node-newrelic that referenced this pull request Jul 26, 2024
feat: Added a shim to externalize all 3rd party libraries the Node.js agent instruments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants