-
Notifications
You must be signed in to change notification settings - Fork 111
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
RSDK-914 - Implement session manager and client support #1630
Conversation
FYI @stevebriskin: not required for review but add yourself if you'd like! |
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.
looks good but definitely want to see some tests
@@ -146,7 +146,7 @@ func TestBaseRemoteControl(t *testing.T) { | |||
}, | |||
logger) | |||
test.That(t, err, test.ShouldBeError, | |||
errors.New("dependency \"baseTest\" should an implementation of <nil> but it was a *inject.InputController")) | |||
errors.New("dependency \"baseTest\" should be an implementation of base.Base but it was a *inject.InputController")) |
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.
@Kschappacher fyi
@@ -121,17 +121,17 @@ var ( | |||
|
|||
// NewUnimplementedInterfaceError is used when there is a failed interface check. | |||
func NewUnimplementedInterfaceError(actual interface{}) error { | |||
return utils.NewUnimplementedInterfaceError((Arm)(nil), actual) | |||
return utils.NewUnimplementedInterfaceError((*Arm)(nil), actual) |
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.
@cheukt this was not working
Name string `json:"name"` | ||
Address string `json:"address"` | ||
Frame *Frame `json:"frame,omitempty"` | ||
Auth RemoteAuth `json:"auth"` | ||
ManagedBy string `json:"managed_by"` | ||
Insecure bool `json:"insecure"` | ||
ConnectionCheckInterval time.Duration `json:"connection_check_interval,omitempty"` | ||
ReconnectInterval time.Duration `json:"reconnect_interval,omitempty"` | ||
ConnectionCheckInterval string `json:"connection_check_interval,omitempty"` |
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.
@cheukt neither was this
|
See JIRA for a copy of the scope. This is ready to review but still needs: