-
Notifications
You must be signed in to change notification settings - Fork 596
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
WC-1473: Add support for script content and script settings read/update endpoints #1361
Conversation
changelog detected ✅ |
Codecov Report
@@ Coverage Diff @@
## master #1361 +/- ##
==========================================
+ Coverage 48.33% 48.42% +0.08%
==========================================
Files 133 135 +2
Lines 13023 13303 +280
==========================================
+ Hits 6295 6442 +147
- Misses 5201 5291 +90
- Partials 1527 1570 +43
|
looks good in general, just a couple of notes from https://github.com/cloudflare/cloudflare-go/blob/master/docs/conventions.md#methods that we should update here for consistency. |
thanks @echen67, this looks awesome. once we have those API schemas land, we're good to go here. |
return "", ErrMissingAccountID | ||
} | ||
|
||
uri := fmt.Sprintf("/accounts/%s/workers/scripts/%s/content/v2", rc.Identifier, scriptName) |
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.
the API docs suggest this is just /accounts/:account_id/workers/scripts/:script/content
. is that correct?
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.
Ah, technically both work, but /v2 is preferred for this GET endpoint so I'll update the API docs to match. Thanks for catching this!
thanks for getting this one over the line @echen67 🚀 |
This functionality has been released in v0.76.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
This PR is to support new endpoints that allow the user to read/update either only the script content or only the script settings of a worker, as opposed to currently having to always update everything together. This makes it easier on the user when they only need to deal with one aspect of their worker.
Has your change been tested?
I have added new tests to cover this change.
Types of changes
What sort of change does your code introduce/modify?
Checklist:
and relies on stable APIs.