-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat: Add bootstrap handler to create Messaging Client with secure options #225
Conversation
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.
Overall LGTM, just some minor tweaks
Codecov Report
@@ Coverage Diff @@
## master #225 +/- ##
==========================================
+ Coverage 52.47% 58.00% +5.52%
==========================================
Files 11 12 +1
Lines 646 750 +104
==========================================
+ Hits 339 435 +96
- Misses 293 300 +7
- Partials 14 15 +1
Continue to review full report at Codecov.
|
Unit test failed. |
…tions Close #224 Signed-off-by: lenny <[email protected]>
Signed-off-by: lenny <[email protected]>
Signed-off-by: lenny <[email protected]>
@bnevis-i , THX! Fixed. |
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.
lgtm
bootstrap/messaging/messaging.go
Outdated
defer wg.Done() | ||
select { | ||
case <-ctx.Done(): | ||
_ = msgClient.Disconnect() |
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.
it would be safer to write
if (msgClient != nil) {
_ = msgClient.Disconnect()
}
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.
Agreed.
Signed-off-by: lenny <[email protected]>
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.
LGTM
Signed-off-by: lenny <[email protected]>
Other upcoming PRs for SDKs/Services and Core Data depend on this PR.
PR Checklist
Please check if your PR fulfills the following requirements:
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-bootstrap/blob/master/.github/Contributing.md.
What is the current behavior?
No common bootstrap handler exists
Issue Number: #224
What is the new behavior?
Common bootstrap handler now exists
Does this PR introduce a breaking change?
New Imports
Specific Instructions
Are there any specific instructions or things that should be known prior to reviewing?
Other information