feat(sdk): init Go, Python and TypeScript SDKs#4358
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // | ||
| // The `asc` suffix is optional as the default sort order is ascending. The `desc` | ||
| // suffix is used to specify a descending order. | ||
| type SortQuery struct { |
There was a problem hiding this comment.
This looks incorrect to me, since the sort should be string type.
|
|
||
| sdk := sdkpkg.New(sdkpkg.WithServerURL(srv.URL)) | ||
| ctx := context.Background() | ||
| res, err := sdk.OpenMeterMeters.ListMeters(ctx, operations.ListMetersRequest{}) |
There was a problem hiding this comment.
The method naming is redundant, can you please change it? Either way would be great, like: sdk.OpenMeterMeters.List(...) or sdk.OpenMeter.ListMeters(...)
I personally like the first option a little bit more.
| } | ||
|
|
||
| // String provides a helper function to return a pointer to a string | ||
| func String(s string) *string { return &s } |
There was a problem hiding this comment.
This part is redundant, since the types package already has a pointer helper file. Can you please use that instead of these?
| { | ||
| "port": "8888", | ||
| }, | ||
| {}, |
There was a problem hiding this comment.
Can you please remove the empty map instances from here?
| return nil, fmt.Errorf("error generating URL: %w", err) | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "GET", opURL, nil) |
There was a problem hiding this comment.
Can you please modify the skill to use http.MethodGet and other constants for method setup?
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var out map[string]any |
There was a problem hiding this comment.
This way the response value would be nearly typeless, which is not a good thing, can you please force it to unmarshal into the real response format? Also, this is not a very optimal way for parsing an io.Reader, please use the json.NewDecoder(httpRes.Body).Decode(res.Body) instead.
|
|
||
| type ListAddonsResponse struct { | ||
| HTTPMeta components.HTTPMetadata | ||
| Body *map[string]any |
There was a problem hiding this comment.
Please force the generator to populate here the real ListAddon response format.
| if res == nil { | ||
| t.Fatal("ListMeters returned nil response") | ||
| } | ||
| if res.HTTPMeta.Response == nil { |
There was a problem hiding this comment.
Smoke test it not correct, please change it to check the type of the response body.
tothandras
changed the title
2 participants
No description provided.