Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Service Bus] [Core AMQP] Merge AmqpMessage{Header|Properties} exported from both the libraries #12091
[Service Bus] [Core AMQP] Merge AmqpMessage{Header|Properties} exported from both the libraries #12091
Changes from 18 commits
4e20516
9b8b905
36ca283
7a7b701
8a4e6d4
bcf7a0e
30f16f1
11f412d
f510cae
ef92276
779aa91
132d942
dc691cf
6392a44
b0b94a6
35a38ec
43f0396
ad6da86
37ed73a
6786692
a745549
e87d659
dfe570a
ba70742
1f0cde3
8738f03
cf1eab8
e90c03f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Can we keep this as
Message
and not use theAmqp
prefix here? Everywhere else where we use theAmqp
prefix, we are referring to the type/interface defined in core-amqp which has camel casing. Where as thisAmqpMessage
is from rhea-promise. To be consistent withMessageHeader
andMessageProperties
from rhea-promise, this should justMessage
as wellThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We either do that or use the
Rhea
prefixThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking it, but refrained from doing since it would change service-bus and event-hubs.
But it is probably worth doing it.. will do!