In-Touch / newrelic

Composer Package for NewRelic Wrapper
37 stars 11 forks source link

Enhancement: Use handlers to make actual calls #14

Closed localheinz closed 9 years ago

localheinz commented 9 years ago

This PR

:information_desk_person: The idea behind extracting a handler is that we both want to assert that the right functions are called with the right arguments, as well as be able to use a different handler in environments where we don't want to send messages to the New Relic API even if the extension is installed (for example, in development, testing, and / or staging environments).

pleckey commented 9 years ago

@localheinz we're actually looking to hire a full-time, permanent SDET - any interest? or know anyone? will consider remote for the right person ... :wink:

localheinz commented 9 years ago

@pleckey

Ha, sounds great, but I'm very happy at the moment!

Regarding this PR, if you like, I can break it apart. Overall, all of the changes should not break any behaviour - that is, unless you don't install this package with Composer (auto-loading, eh?).

If you consider this worth merging, we should probably update README.md, too?

Maybe this is useful for https://github.com/In-Touch/laravel-newrelic, too?

pleckey commented 9 years ago

@localheinz This is awesome, thank you (especially for the tests). I have some changes to the L5 package I want to get up, at which point I will tag new releases for both as current stable.

localheinz commented 9 years ago

@pleckey

Nice, sounds great!

I was thinking, maybe considering a new major release where we move the parameter $throw to the actual handler?

pleckey commented 9 years ago

Probably a good idea. I'll push up a 2.0 branch and we can start making changes from there, merge it into master when it's ready.

pleckey commented 9 years ago

@localheinz 2.0 branch is there now - was also thinking, would be a good target for your PSR-4 PR as well.

localheinz commented 9 years ago

@pleckey

:+1: