-
Notifications
You must be signed in to change notification settings - Fork 6
docs: clarify additional steps for adding a new worker #582
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,32 @@ Create a new directory `<worker-name>/` with: | |
| 4. **`.github/workflows/ci.yml`** - Add deployment step in the `deploy` job | ||
| 5. **`CLAUDE.md`** - Add the new worker to the "Monorepo Structure" list | ||
|
|
||
| ## Worker Registration | ||
|
|
||
| After creating the worker directory, the worker must be registered in the repository so it is discoverable by tooling and CI. | ||
|
|
||
| Update the worker registry file to include the new worker name: | ||
|
|
||
| - `workers.ts` (or equivalent registry file) | ||
|
|
||
| Add the worker name to the exported list/map following the existing pattern. | ||
|
|
||
|
|
||
| 4. **`.github/workflows/ci.yml`** | ||
| - Add the worker name to the deploy matrix | ||
| - Ensure calibration and mainnet deployments include the new worker | ||
| - Follow the existing worker entries as a template | ||
|
Comment on lines
+68
to
+71
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already covered by item 4 in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged — I’ll remove the duplicate CI section. |
||
|
|
||
|
|
||
| ## Recommended Approach | ||
|
|
||
| It is strongly recommended to copy an existing worker directory and modify it rather than creating files from scratch. This helps ensure: | ||
|
|
||
| - Correct Wrangler configuration | ||
| - Consistent scripts | ||
| - Proper CI integration | ||
|
Comment on lines
+74
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This advice is wrong. We are moving from JS-based to TS-based workers; we don't want to copy old JS-based workers when creating a new one.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood — thank you for clarifying. I’ll remove the recommendation about copying existing workers and adjust the wording to align with the TS-based workflow. |
||
|
|
||
|
|
||
| ## Verification Steps | ||
|
|
||
| ```bash | ||
|
|
@@ -101,3 +127,10 @@ If your worker needs access to the shared D1 database: | |
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Common Pitfalls | ||
|
|
||
| - Forgetting to register the worker in the worker registry | ||
| - Missing CI deploy configuration | ||
| - Including local-only changes (e.g. package-lock.json) | ||
| - Forgetting to run `update-ts-project` | ||
|
Comment on lines
+131
to
+136
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this section needed for?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense — I’ll remove the Common Pitfalls section to keep the document focused. |
||
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 does make sense to me. What worker registry file do you have in mind?
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.
You’re right — there is no explicit worker registry file. That was an assumption on my part. I will remove this section.
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.
Thanks @bajtos for the feedback — my goal is to help contributors who add a new worker manually (not via Claude) avoid missing required updates that caused issues in #375. I’ll narrow the scope of this PR to only document steps that are not already covered and remove anything speculative