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

Add noop implementation of newrelic.Application and newrelic.Transaction #2

Closed

Conversation

v-yarotsky
Copy link

Useful for tests and in cases where it's impossible to initialize the
actual newrelic.Application instance, but the app needs to run
regardless.

I can add it to our wrapper library, but I thought it might be useful to others as well.

Useful for tests and in cases where it's impossible to initialize the
actual newrelic.Application instance, but the app needs to run
regardless.
@jairodasilva
Copy link

I don't think this is necessary. You can use counterfeiter to generate your fakes. I use this tool in my project to fake/mock the Application and Transaction interfaces.

@willnewrelic
Copy link

Hi v-yarotsky

Thanks for the PR. We appreciate your desire to contribute.

For legal purposes, we have just updated the LICENSE.txt with additional information about contributions. Please take a look. As a result, I'm going to have to close this PR and ask you to resubmit it. I'm sorry for the inconvenience.

@purple4reina
Copy link
Contributor

Hello @v-yarotsky!

We've recently published some proposed changes to the agent for a 3.0 major version release #106. We're wondering what you think of these changes and if they will fit your needs.

Particularly, we're proposing changing newrelic.Application and newrelic.Transaction from interfaces to structs. This would remove the requirement for nil checks throughout your code and/or a noop Application and Transaction. Please take a look at it and let us know what you think!

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.

None yet

4 participants