-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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] Merge feature/service-bus-track-2 into master #7312
[Service Bus] Merge feature/service-bus-track-2 into master #7312
Conversation
…nto harsha/sb/merge-to-master
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
Live tests - Build ✔ green |
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.
Left a few comments, please take a look along with resolving merge conflicts
@HarshaNalluru After addressing the review comments and resolving the merge conflicts, please trigger the live tests |
…to harshan/sb/merge-to-master
/azp run js - servicebus - tests |
Pull request contains merge conflicts. |
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Just have one comment about deps.
@@ -90,11 +88,12 @@ | |||
"long": "^4.0.0", | |||
"process": "^0.11.10", | |||
"rhea": "^1.0.18", | |||
"rhea-promise": "^0.1.15", | |||
"rhea-promise": "^1.0.0", |
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.
This and rhea
shouldn't be needed anymore. core-amqp
pulls them in.
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.
removed rhea
and rhea-promise
.
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.
Sorry, I didn't realize rhea-promise was being used directly. It looks like only rhea can be safely removed.
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.
added "rhea-promise" back
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Assuming the live tests pass, 🚢🇮🇹
Dismissing since approved b y Chris
This PR is to merge
feature/service-bus-track-2
branch intomaster
.