-
Notifications
You must be signed in to change notification settings - Fork 26
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
Possibility to use service as person_fn rather than a static function #93
base: master
Are you sure you want to change the base?
Possibility to use service as person_fn rather than a static function #93
Conversation
@zinkovskiy, we are looking for a maintainer or owner of the repo. Would you like to participate? |
Hi @zdavis-rollbar yes, I'm ready to maintain. contact me anytime when you have time, we are interested in keeping the project up to date |
@zinkovskiy Thank you for the PR. Please add tests for the new functionality, and run tests locally to ensure CI will pass. |
Hi @waltjones, yes, I missed it. Now it's added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Anton, thank you for your work on this and your patience in getting this reviewed!
I appreicate how customizable and extensible you have made this. Great work!
Description of the change
This PR allow to use service as
person_fn
I created PersonProvider service that is used by default
To use custom person provider you need to define your own service with alias
rollbar.person-provider
in services.yml of your application, for example:No need to define
person_fn
option (thx to DI), the service will be used automatically, but ifperson_fn
is defined, then it will be usedLogic of obtaining user info is not changed.
I've also discovered, that
security.token_storage
andserializer
are private and cannot be obtained from container, so I passed it toRollbar\Symfony\RollbarBundle\Factories\RollbarHandlerFactory
constructorType of change
Related issues
Checklists
Development
Code review
I tested these changes on symfony 5.4 and symfony 6.4, it works well
@danielmorell could you please take a look on it?