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] Support cancellation on $management operations #8159
[Service Bus] Support cancellation on $management operations #8159
Changes from 4 commits
e7809ea
cf46a27
bec36f4
c1a0e44
6cbdf74
c064211
ab06ab1
0d9bc9e
41703cd
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.
I don't think I've ever noticed how funny this looks.
Also, now it makes me wonder why SpanOptions doesn't contain a field called spanOptions. :)
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.
Yeah, personally I think I prefer just putting the
tracingOptions?
field on whatever currently extendsTracingOptions
, since this only has one field and is now exposed to users when it probably wouldn't be used otherwise. But that's just my opinion 😄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.
Yeah, @ramya-rao-a 's made this argument in the past and I keep going back and forth on it. I mostly just worry about accidentally making it inconsistent but we can probably counter that by simply having a compile-test like you did recently for core-amqp (I believe)?
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.
Any change we want to do here is something I want to ensure is the same between Event Hubs and Service Bus. We already have AMQP and HTTP packages diverging a little, I would prefer to have as much common coding and design patterns between Service Bus and Event Hubs for our own sanity when we move between these 2 projects when in maintainence mode