Adds ip-traceable listener #233
Conversation
|
+1 |
| @@ -49,6 +50,9 @@ public function load(array $configs, ContainerBuilder $container) | |||
| } elseif ('blameable' === $ext) { | |||
| $useBlameable = true; | |||
| } | |||
There was a problem hiding this comment.
Thanks for notice. This is fixed. 👍
|
+1 |
|
@stof Any opinion about this PR ? |
| * | ||
| * @author Pierre-Charles Bertineau <pc.bertineau@alterphp.com> | ||
| */ | ||
| class BlameListener implements EventSubscriberInterface |
There was a problem hiding this comment.
Should be IpTraceListener according to your configuration (and file name), right?
|
👍 but it needs some cleaning, see comments. |
| */ | ||
| public function onKernelRequest(GetResponseEvent $event) | ||
| { | ||
| $ip = $event->request->getClientIp(); |
There was a problem hiding this comment.
You can skip this event if the current request isn't a master request.
|
+1 |
3 similar comments
|
+1 |
|
+1 |
|
+1 |
|
@stof Can you review this PR ? |
|
👍 |
|
Hello @stof I know you're working on many other projects, I heard about on php-cs-fixer and symfony... But can you review this one please ? |
alterphp
force-pushed
the
iptraceable-listener
branch
from
1f6298d
to
727276b
Compare
|
Hey @stof : any review ? |
|
+1 |
alterphp
force-pushed
the
iptraceable-listener
branch
from
3677391
to
8dcfe4b
Compare
|
UP |
|
👍 Please merge this ASAP... |
|
+1 |
1 similar comment
|
+1 |
| @@ -24,6 +25,11 @@ | |||
| <argument type="service" id="annotation_reader" /> | |||
| </call> | |||
| </service> | |||
| <service id="stof_doctrine_extensions.listener.ip_traceable" class="%stof_doctrine_extensions.listener.ip_traceable.class%" public="false"> | |||
There was a problem hiding this comment.
please do not add a parameter for the service class. This is discouraged in Symfony now
alterphp
force-pushed
the
iptraceable-listener
branch
from
2751ce3
to
5ef846f
Compare
|
@stof I made some updates in order to enhance Symfony compliance as you told me (even if it breaks bundle uniformity). Please tell me... |
|
I'm still 👍 on this feature, now the implementation seems perfect I think it really can be merged 👍 |
|
+1 |
|
Why this has not been merged yet? any reason? |
|
@alfonsomga Because this bundle doesn't seem to be maintained anymore... If you have some time, there are several projects put forward to improve the new library. |
|
Hey @stof, Any chance you could take a peek & hopefully merge this PR? Been a while :/ Thanks! |
|
Hey @stof , Thank you for your time. |
|
KnpLabs looks more maintained, but there are not ip_traceable too |
|
+1 |
|
Nearly 2 years since my last commit... I hope you don't mind if I wait for @stof confirmation before doing anything... |
|
...and I stumbled upon this in April 2018, still not merged. Come on... |
|
why it can not be merged ? |
|
It's pretty obvious that stof has no time to maintain this repository more than just "compatiblity with new Sf versions and security fixes". So if somebody is not happy with that, you can probably enter in discussion in order to fork the repository and maintain it. For example under friendofsymfony organization. Feel free to send some mails to take the temperature of people on the topic. In any case more talk here is just spam for maintainer(s). |
|
Any update? |
|
I put this Gist together to help anyone looking to integrate IpTraceable in a Symfony 5.4 app. |
|
Replaced by #453 as the config of the bundle changed too much in the meantime. |
Adds support for IpTraceable Gedmo extension