Auto on session tracking #517
Conversation
There was a problem hiding this comment.
Lets chat tomorrow because I think I'm missing some context on the changeset. If you think its sensible to decouple some of these changes then feel free to go ahead and make any changes you see fit, but if i just need more context on why this is the right way we can chat tomorrow!
UPGRADING.md
Outdated
| ); | ||
| ``` | ||
|
|
||
| #### Configuring the GuzzleHttp Client |
There was a problem hiding this comment.
Why is this change required for always on session tracking?
UPGRADING.md
Outdated
| $bugsnag->setGuzzleClient($myCustomGuzzle); | ||
| ``` | ||
|
|
||
| #### Deprecation |
There was a problem hiding this comment.
Why is this change required for always on session tracking?
There was a problem hiding this comment.
This isn't, but should be removed if this is a major release. I've removed it as it'll be a part of a release PR instead.
UPGRADING.md
Outdated
| @@ -1,14 +1,78 @@ | |||
| Upgrading | |||
| ========= | |||
|
|
|||
| ## 3.x to 4.x | |||
There was a problem hiding this comment.
Rather than bumping the major version why not support backwards compat?
| * | ||
| * @return void | ||
| */ | ||
| public function __construct(Configuration $config, ClientInterface $guzzle) | ||
| public function __construct(Configuration $config) |
There was a problem hiding this comment.
why has passing guzzle in been deprecated?
|
I've removed the non-pertinent changes that were a part of this PR originally, and attempted to clarify a number of the questions that require more context. |
|
This is on hold for now, and will require review and possibly redevelopment in the future |
Goal
trueby defaultClient/Configurationafter initialization.endpointcould only be set during initialization by its inclusion in the GuzzleHttp\Client. As the Guzzle client is immutable after instantiation, this meant that any method of ensuring that the notify and session endpoints were both set would be needlessly complex.setEndpoint, it allows the same logic that's been used across the other notifiers for setting endpoints to be utilized.