Skip to content

Conversation

@mattrpav
Copy link
Contributor

@mattrpav mattrpav commented Feb 16, 2022

  • Create PR for code change
  • Update website network of brokers page to document the change

@mattrpav mattrpav requested a review from cshannon February 16, 2022 20:09
@mattrpav mattrpav self-assigned this Feb 16, 2022
@cshannon
Copy link
Contributor

This change itself seems fine but I'm not sure if any existing network bridge tests will break. So you need to check all the network tests and run them to see if anything doesn't work now that the default changed.

Also, yes the change is trivial but again, all changes should have tests. You don't need to write anything crazy but you could at least write a test to verify the new default is true (could just check the network connector/bridge or even use the JMX NetworkConnectorView after start up) to prevent getting broken in the future.

@mattrpav
Copy link
Contributor Author

@cshannon yep. Plan to. I kicked in these PR’s to get the tests running in parallel to see fallout. I’ll be going through the unit test failures and adding the default value tests next

@mattrpav
Copy link
Contributor Author

Note-- first test run failed on a StompTransport test (appears unrelated). Kicking off fresh run of tests

@mattrpav
Copy link
Contributor Author

All tests passed once Jenkins cleared up due to maint yesterday. I'll be adding a couple tests to verify default settings

@mattrpav mattrpav merged commit 413dfa9 into apache:main Mar 14, 2023
@mattrpav mattrpav deleted the AMQ-5137 branch March 14, 2023 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants