Support for attaching a Worker to a Domain#1014
Support for attaching a Worker to a Domain#1014jacobbednarz merged 5 commits intocloudflare:masterfrom
Conversation
|
changelog detected ✅ |
Codecov Report
@@ Coverage Diff @@
## master #1014 +/- ##
==========================================
+ Coverage 49.06% 49.72% +0.66%
==========================================
Files 108 115 +7
Lines 10428 10862 +434
==========================================
+ Hits 5116 5401 +285
- Misses 4200 4299 +99
- Partials 1112 1162 +50
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
a2c37b8 to
ff5e502
Compare
|
Working on tests next. @jacobbednarz should I switch the arguments to be inside a struct? Some of the methods use a strict while others take direct arguments. I'm leaning towards switching to the struct pattern. Let me know! |
|
check out https://github.com/cloudflare/cloudflare-go/blob/master/docs/experimental.md, #1028, and https://github.com/cloudflare/cloudflare-go/blob/master/tunnel.go for prior art on how we should be adding new methods. |
|
@jacobbednarz Updated per your examples, please review! It seems like In workers.go, I followed the example of DeleteWorker, DownloadWorker, etc. that have ZoneID in the respective params structs. |
there are currently no other implementation of going forward, we want to use the updated method signature for all new methods to help future proof against these issues. |
Just to be clear, does that mean I should add the new method using the new style in a manner differently than my latest commit, or that you'll circle back to workers.go at a later date and redo all the method signatures in one go? PS - I would expect to squash all commits in this PR, let me know if you'd like to do that as part of a review and merge or if I should force push my branch. I am very comfortable with rebase workflow, so this is no hardship at my end. |
sorry if it wasn't clear. you should do this (i'll add a note inline as a reminder).
no need, i'm happy to merge/squash at the end. |
| attachWorkerToDomainResponse = `{ | ||
| "result": { | ||
| "id": "e7a57d8746e74ae49c25994dadb421b1", | ||
| "zone_id":"foo", |
There was a problem hiding this comment.
you'll want to use testZoneID here instead.
There was a problem hiding this comment.
Need to move out of the const block to the var block in order to use fmt.Sprintf here
9f54d25 to
ee5e98e
Compare
|
thanks! appreciate the effort here! |
|
Thank you! |
|
This functionality has been released in v0.47.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Description
Support for the new Custom Domain API at https://api.cloudflare.com/#worker-domain-attach-to-domain
Has your change been tested?
Works for me locally, unit tests will be added in PR review.
Screenshots (if appropriate):
Types of changes
What sort of change does your code introduce/modify?
Checklist: