Update client package docs #107
Conversation
Codecov Report
@@ Coverage Diff @@
## main #107 +/- ##
==========================================
+ Coverage 75.77% 76.24% +0.46%
==========================================
Files 22 22
Lines 1577 1633 +56
==========================================
+ Hits 1195 1245 +50
- Misses 288 292 +4
- Partials 94 96 +2
Continue to review full report at Codecov.
|
client/httpclient.go
Outdated
| @@ -33,6 +34,7 @@ func NewHTTP(logger types.Logger) *httpClient { | |||
| return w | |||
| } | |||
|
|
|||
| // Start starts the client and runs until Stop is called. | |||
There was a problem hiding this comment.
Start() is explained in more detail in the OpAMPClient interface description. We can either copy the comment from there or reference from here.
There was a problem hiding this comment.
Right, I wasn't really sure about this. What do you think about remaining ones SetAgentDescription, SetHealth etc..? Should we copy for all of them?
There was a problem hiding this comment.
If you can add a reference in a way that it becomes a link in godoc here that would be best, so that we don't have to duplicate the content and risk diverging.
There was a problem hiding this comment.
Looks like there is no way to achieve this. I looked up how the packages in stdlib cross reference the functions from other modules. It just uses plain text like os.Exit or json.Marshal for the same reason.
There was a problem hiding this comment.
OK, let's just put plain text to reference OpAMPClient.Start.
client/types/callbacks.go
Outdated
| @@ -39,6 +39,8 @@ type MessageData struct { | |||
| } | |||
|
|
|||
| type Callbacks interface { | |||
| // OnConnect is called when the connection is successfully established to the Server. | |||
| // May be called after Start() is called and every time connected to the Server. | |||
There was a problem hiding this comment.
| // May be called after Start() is called and every time connected to the Server. | |
| // May be called after Start() is called and every time a connection is established to the Server. |
There was a problem hiding this comment.
Should we add more description here for websocket and http?
For example:
WebSocket clients will call OnConnect when a new WebSocket connection is established.
HTTP clients will call OnConnect after any request that results in a 200 response.
There was a problem hiding this comment.
Good suggestion. I will update the description.
Co-authored-by: Andy Keller <andykellr@users.noreply.github.com>
Partially addresses #100. Also added to some internal funcs which I thought would be helpful to get a quick summary.