-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add per call options to ygnmi #57
Conversation
Pull Request Test Coverage Report for Build 2892189026
💛 - Coveralls |
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, some minor comments
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.
Thanks for the thorough tests
stub *Stubber | ||
agent *gnmi.Agent | ||
stub *Stubber | ||
getWrapper *getWrapper |
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 know this was the Ondatra naming, but maybe "clientWrapper" would be a better name, since it wraps the client? Or perhaps the field could be named "clientWrapper" and the struct named "clientWithGetter"
getRequests []*gpb.GetRequest | ||
} | ||
|
||
// Get is fake implement of gnmi.Get, it returns the GetResponse contained in the stub. |
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.
"is fake implement" -> "is a fake implementation"?
@@ -87,6 +116,12 @@ func (s *Stubber) Notification(n *gpb.Notification) *Stubber { | |||
return s | |||
} | |||
|
|||
// AppendGetResponse appends the given GetResponse as a stub response. | |||
func (s *Stubber) AppendGetResponse(gr *gpb.GetResponse) *Stubber { |
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.
All the other methods on Stubber are named by what they stub/append and omit the word "append". Justification for this one breaking that pattern?
return nil | ||
} | ||
|
||
// Recv returns the result of the Get request, returning io.EOF after resonpse are read. |
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.
*response
fixes in #58 |
No description provided.