You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/09/23 13:17:37 UTC
[GitHub] [activemq] ehossack-aws opened a new pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
ehossack-aws opened a new pull request #714:
URL: https://github.com/apache/activemq/pull/714
As suggested in #679 , this adds a log toggle config to allow users to opt back in to the non-inclusive terminology previously used, as the messages may be used in monitoring/alerting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] mattrpav commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
mattrpav commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-1020293926
Please hold on merge, need to review the terminology.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-1020379094
To be honest, I'm not a big fan of having a configuration for that.
I think we can just go for the change for ActiveMQ 5.17.x and keep as it is on 5.16.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] ehossack-aws commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-925808835
Not super sure the best way to chain commits without a target branch, so linking the new commit in this pr:
https://github.com/apache/activemq/pull/714/commits/3f7eb0a39ba3e7926e72f91fa3f9f6748e261726
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-1020379094
To be honest, I'm not a big fan of having a configuration for that.
I think we can just go for the change for ActiveMQ 5.17.x and keep as it is on 5.16.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] ehossack-aws commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
ehossack-aws commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-951093018
> It looks good to me and a good idea. Can you please squash the three commits ? Thanks
Can we merge https://github.com/apache/activemq/pull/700 first? That is the parent to this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] jbonofre commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
jbonofre commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-950524651
It looks good to me and a good idea. Can you please squash the three commits ? Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon commented on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
cshannon commented on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-1055749169
Agree with @jbonofre . I think having a configuration makes no sense. Just change it but at this point it should be target for 5.18 as 5.17 should be frozen except for critical stuff
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq] cshannon edited a comment on pull request #714: AMQ-8317 add config param to toggle inclusive terms in logs
Posted by GitBox <gi...@apache.org>.
cshannon edited a comment on pull request #714:
URL: https://github.com/apache/activemq/pull/714#issuecomment-1055749169
Agree with @jbonofre, I think having a configuration option makes no sense. ~~Just change it but at this point it should be target for 5.18 as 5.17 should be frozen except for critical stuff~~ I think we can include it in 5.17.0 since it's just logging if you want to but in my opinion it should have the toggle/flag option removed as discussed on the dev list thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org