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.

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

Make RabbitServer's Queues and Exchanges public. #115

Merged
merged 1 commit into from
Nov 24, 2022

Conversation

Quogu
Copy link

@Quogu Quogu commented Nov 23, 2022

Consequently make other types public that would otherwise result in compile errors due to inconsistent visibilities.

The motivation for this is that without exposing the server's Queues and Exchanges, it's impossible (or at least impractical to the point where I don't understand how) to assert about state in unit tests, which renders the library unusable for its intended purpose.

Consequently make other types public that would otherwise result in compile errors due to inconsistent visibilities.

The motivation for this is that without exposing the server's Queues and Exchanges, it's impossible (or at least impractical to the point where I don't understand how) to assert about state in unit tests, which renders the library unusable for its intended purpose.
@odalet
Copy link

odalet commented Nov 24, 2022

By reading your PR title, I was at first concerned this would leak implementation details to the output world. My concen here is that integrating future versions of the RabbitMQ client might require breaking changes in those.
However, your rationale for this is legit: I never had any need for this in my own case (yet), but I guess other users have other needs.
So, if we need to break things in the future, so be it... Fine by me.

@odalet odalet merged commit 22df4c8 into addupsolutions:master Nov 24, 2022
@Quogu Quogu deleted the PublicRabbitServerProps branch December 6, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants