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.

Sign up for GitHub

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

Jump to bottom

AMQ-8317 add config param to toggle inclusive terms in logs #714

Closed
wants to merge 3 commits into from
Closed

AMQ-8317 add config param to toggle inclusive terms in logs #714

wants to merge 3 commits into from

Conversation

ehossack-aws
Copy link
Contributor

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.

@ehossack-aws
[AMQ-8316] Remove unused BrokerService methods
cc763a9 
This removes passiveSlave and waitForSlave
and waitForSlaveTimeout which are no longer used
@ehossack-aws
[AMQ-8390] Deprecate configuration that uses non-inclusive language
e34ae17 
This is the first in a series of changes that will phase out
non inclusive terminology throughout the config, and codebase.
@ehossack-aws
[AMQ-8317] Add config params for non-inclusive logging
3f7eb0a 
This commit adds the parameter 'useNonInclusiveTerminologyInLogs'
as an opt-in backwards-compatible option for those that
rely on the legacy, non-inclusive terminolgy logged by
the Broker and the lease locker.
@ehossack-aws
Copy link
Contributor Author

Not super sure the best way to chain commits without a target branch, so linking the new commit in this pr:
3f7eb0a

@jbonofre jbonofre self-requested a review September 27, 2021 16:43
@jbonofre
Copy link
Member

It looks good to me and a good idea. Can you please squash the three commits ? Thanks

@ehossack-aws
Copy link
Contributor Author

It looks good to me and a good idea. Can you please squash the three commits ? Thanks

Can we merge #700 first? That is the parent to this PR.

@mattrpav mattrpav self-requested a review January 24, 2022 16:34
@mattrpav mattrpav self-assigned this Jan 24, 2022
@mattrpav
Copy link
Contributor

Please hold on merge, need to review the terminology.

@jbonofre
Copy link
Member

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.

@cshannon
Copy link
Contributor

cshannon commented Mar 1, 2022 •
edited

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.

@ehossack ehossack mentioned this pull request Mar 1, 2022
Closed
This was referenced Apr 28, 2022
Closed
Open
Open
Open
@jbonofre
Copy link
Member

It will superseded by another PR for 7.0.0 with directly terminology change.

@jbonofre jbonofre closed this Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbonofre jbonofre jbonofre approved these changes

@mattrpav mattrpav Awaiting requested review from mattrpav

Assignees

@mattrpav mattrpav

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@ehossack-aws @jbonofre @mattrpav @cshannon