-
Notifications
You must be signed in to change notification settings - Fork 597
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
Remove double-unmarshalling #1046
Conversation
changelog detected ✅ |
@@ -320,31 +320,20 @@ func (api *API) UpdateTunnelConfiguration(ctx context.Context, rc *ResourceConta | |||
} | |||
|
|||
uri := fmt.Sprintf("/accounts/%s/cfd_tunnel/%s/configurations", rc.Identifier, params.TunnelID) | |||
res, err := api.makeRequestContext(ctx, http.MethodPut, uri, params.Config) |
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.
This is a slightly different bug but also has to do with the marshalling/unmarshalling of the config
param.
I am new to this process so let me know if there's anything that I'm missing. |
let me confirm with the service team as this was the correct API response when it was added. |
tunnel.go
Outdated
@@ -73,9 +73,9 @@ type TunnelConnectionResponse struct { | |||
// other than in the HTTP response unmarshaling. Use `TunnelConfigurationResult` | |||
// for the correct types. | |||
type TunnelConfigurationStringifiedConfigResult struct { |
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.
this should be renamed now to better reflect it's use
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 think it can be removed now (see my latest PR).
I think this is OK. It's not safe since it can be breaking because TunnelConfigurationStringifiedConfigResult
was an exported type before in the SDK but I doubt anyone was directly referencing those types.
Codecov Report
@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
+ Coverage 49.06% 49.94% +0.88%
==========================================
Files 108 115 +7
Lines 10428 10991 +563
==========================================
+ Hits 5116 5490 +374
- Misses 4200 4338 +138
- Partials 1112 1163 +51
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
this still needs a CHANGELOG too |
Co-authored-by: Jacob Bednarz <[email protected]>
thanks for getting this over the line 🎉 |
@jacobbednarz - Thanks for helping with this PR. Do you know when you will cut a new release so that I can switch over to the official version instead of my fork? |
details in https://github.com/cloudflare/cloudflare-go/blob/master/docs/release-process.md. you can also check the milestone that this PR was added to. |
This functionality has been released in v0.48.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Description
Fixes #1045
Has your change been tested?
Modified existing tests and ran
go test
.Screenshots (if appropriate):
Types of changes
What sort of change does your code introduce/modify?
Checklist: