Feature/heh alternate exchanges #36
Conversation
|
PR for issue: #33 |
| @@ -71,14 +70,14 @@ public async Task CanUseAlternateCustomExchangeName() | |||
| var topicWithAlternateExchange = $"{topic}@{alternateExchange}"; | |||
|
|
|||
| var subscriberAddresses = await rabbitMqTransport.GetSubscriberAddresses(topicWithAlternateExchange); | |||
| subscriberAddresses[0].Should().Be(topicWithAlternateExchange); | |||
| Assert.IsTrue(subscriberAddresses[0] == topicWithAlternateExchange); | |||
There was a problem hiding this comment.
Just a tiny remark: The Assert.IsTrue(...) syntax should only be used to check something that is actually a bool. In this case, if the strings are not equal, the error will just say "expected: true, was: false", which is pretty poor.
If you use
Assert.That(subscriberAddresses[0], Is.EqualTo(topicWithAlternateExchange));then the error will be much better.
Just a tip 😉
There was a problem hiding this comment.
thanks! I'll do the update :)
hansehe
force-pushed
the
feature/heh-alternate_exchanges
branch
from
f6e3256 to
6a22d1f
Compare
|
Seems like the integration test failed due to a fail with building the rabbitMq? |
|
Excellent work 👍 – one thing though: What was the reasoning behind "publish on alternate exchanges enabled"? Why not just say that that's the way it always works? |
|
it's out in Rebus.RabbitMq 5.0.0-b08 now 😄 |
|
Well I implemented the 'Enabling' feature to preserve backwards compatibility, since it may be someone out there using an exchange with a '@' letter in the exchange name. Don't really sure if it's possible to have a '@' letter in an exchange name though. Please just remove the 'Enabling' feature in the next release if you don't think it's a problem, and keep it enabled in all cases. |
|
I'll remove it. I don't like too many options 😄 Version 5 is not out yet, so this is a good time to make this change. |
|
it's removed in Rebus.RabbitMq 5.0.0-b09 |
Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.