-
Notifications
You must be signed in to change notification settings - Fork 669
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
MF-898 - Add bulk connect to CLI and SDK #956
Conversation
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #956 +/- ##
==========================================
- Coverage 84% 83.66% -0.34%
==========================================
Files 75 75
Lines 5382 5308 -74
==========================================
- Hits 4521 4441 -80
- Misses 586 595 +9
+ Partials 275 272 -3
Continue to review full report at Codecov.
|
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
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: Nick Neisen <nwneisen@gmail.com>
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!
I'm not sure why the coverage dropped after changing to consts. It's not giving me much information to go off of. |
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
Great contribution again @nwneisen, approved. I'll let other take a look and merge tomorrow. |
Signed-off-by: Nick Neisen <nwneisen@gmail.com>
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
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!
* Add bulk connect to CLI and SDK Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Add test for bulk connect Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Update docs Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Add file example Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Add JSON example to CLI docs Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Change some value datatypes back Signed-off-by: Nick Neisen <nwneisen@gmail.com> * Use constants for file extensions Signed-off-by: Nick Neisen <nwneisen@gmail.com>
What does this do?
Adds bulk connect functionality to the SDK and CLI
Which issue(s) does this PR fix/relate to?
Resolves #898
List any changes that modify/break current functionality
Multiple connections can now be created using the SDK or CLI
Have you included tests for your changes?
Yes
Did you document any new/modified functionality?
Yes
Notes