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

#310 : Add sentry 2.0 type #313

Merged
merged 1 commit into from Aug 13, 2019

Conversation

@Taluu
Copy link
Contributor

commented Aug 8, 2019

As said on #310, sentry completely changed their API and even stopped calling their client "raven", using now php-http to handle the requests to their api.

So this PR adds a new type in the config to support the handler they made.

I'm not sure on a few points (such as the self-referencing hub, which I commented because otherwise it would go on an infinite recursion problem), and I based the service definition on sentry's bundle.

Poke @Jean85 maybe, as he is maintaining (as far as I can tell ?) the current sentry bundle.

Fixes #310

@Taluu Taluu changed the title Add sentry 2.0 type #310 : Add sentry 2.0 type Aug 8, 2019

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch from ecfdbfa to 90b1d18 Aug 8, 2019

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2019

Removed the check for the class on the config (e.g for raven it was going berserk, as the package is not required to make the tests run). I did the same for sentry 2.0's handler.

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch 3 times, most recently from 82d63b0 to c008019 Aug 8, 2019

@lyrixx

lyrixx approved these changes Aug 8, 2019

Copy link
Member

left a comment

👍 LGTM.
We are using sentry on one project I'm working on.
I will test it, and if everything works, I'm gonna merge it.
Thanks

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch from c008019 to 83ab118 Aug 8, 2019

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch 3 times, most recently from aa79e0a to c3e9164 Aug 8, 2019

@Jean85
Copy link

left a comment

This change as is makes the older Sentry no longer compatible. IDK if you want to proceed like this.

Also ping @ste93cry and @HazAT.

@lyrixx

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

This change as is makes the older Sentry no longer compatible. IDK if you want to proceed like this.

yes it is,

  • raven is for the old one
  • sentry is for the new one

Or I missed something?

@Jean85

This comment has been minimized.

Copy link

commented Aug 9, 2019

That's correct, but Sentry 2.0 is pretty recent, so Raven\1.x is still pretty widespread.

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

Then nothing prevents you from using the raven while you're still on the 1.0 ?

I could add something in the doc or somewhere else that says that you can use only one of them at a time, not both.

@lyrixx

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Could you add a note in the CHANGELOG?
It will be part of 3.5.0
Thanks

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch from c3e9164 to 75003bb Aug 13, 2019

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

Done

Show resolved Hide resolved CHANGELOG.md

@Taluu Taluu force-pushed the Taluu:add-sentry-type branch from 75003bb to f91db31 Aug 13, 2019

@lyrixx

lyrixx approved these changes Aug 13, 2019

@lyrixx lyrixx force-pushed the Taluu:add-sentry-type branch from f91db31 to 751173a Aug 13, 2019

@lyrixx

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Thanks for your work on this new feature!

@lyrixx lyrixx merged commit 751173a into symfony:master Aug 13, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

lyrixx added a commit that referenced this pull request Aug 13, 2019

feature #313 #310 : Add sentry 2.0 type (Taluu)
This PR was squashed before being merged into the 3.x-dev branch (closes #313).

Discussion
----------

#310 : Add sentry 2.0 type

As said on #310, sentry completely changed their API and even stopped calling their client "raven", using now php-http to handle the requests to their api.

So this PR adds a new type in the config to support the handler they made.

I'm not sure on a few points (such as the self-referencing hub, which I commented because otherwise it would go on an infinite recursion problem), and I based the service definition on [sentry's bundle](https://github.com/getsentry/sentry-symfony).

Poke @Jean85 maybe, as he is maintaining (as far as I can tell ?) the current sentry bundle.

Fixes #310

Commits
-------

751173a #310 : Add sentry 2.0 type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.