-
Notifications
You must be signed in to change notification settings - Fork 10k
Directly recommend setting remote: true when mentioning remote bindings
#25289
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
Conversation
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
This PR requires additional review attention because it affects the following areas: PartialsThis PR updates partial files, which are pieces of content used across multiple files in our Render component.
|
| ``` | ||
|
|
||
| You can run this script to test it using [remote bindings](/workers/development-testing/#remote-bindings): | ||
| You can run this script to test it via: |
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.
Note: I just removed the remote bindings reference here altogether... to be honest I don't fully follow what this guide is doing with workers AI and why it is using the API directly... 🤔
I think it'd be better not to necessarily include the remote bindings mention here now, revisti/re-evalute the guide, potentially update it and then see where and how to include remote bindings
irvinebroque
left a comment
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.
big thank you for this!
6fd3f02 to
0abd260
Compare
0abd260 to
9b9fa1b
Compare
This PR is for addressing a concern @irvinebroque raised here: #25256 (review)
The fact that the changes in #25256 required users to navigate to different pages instead of directly getting instructions regarding remote bindings.