docs: clarify additional steps for adding a new worker #582
Conversation
FAIZ1409
requested review from
bajtos,
juliangruber and
pyropy
as code owners
bajtos
left a comment
bajtos
left a comment
There was a problem hiding this comment.
Thank you @FAIZ1409 for the pull request.
I am confused about your goal here. The current instructions worked well enough for Claude Code the last time I tried. What is the scenario or use case you are trying to improve?
Also, you are linking to the pull request #374 with the "close" keyword, which does not make sense - the pull request was already merged. Did you perhaps mean to link to a different issue?
| ## 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. |
There was a problem hiding this comment.
This does make sense to me. What worker registry file do you have in mind?
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
This is already covered by item 4 in Files to Update, see above.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| ## 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` |
There was a problem hiding this comment.
What is this section needed for?
There was a problem hiding this comment.
Makes sense — I’ll remove the Common Pitfalls section to keep the document focused.
…worker-doc git reset --hard HEAD# Lines starting with '#' will be ignored, and an empty message aborts
|
While we appreciate your contribution @FAIZ1409 I think you would be better off trying to pick up some of the existing open issues. I propose to you to check out an issue in the issue tab and pick something you'd like, ideally something marked with "good first issue". If the issue is unclear feel free to tag us there with additional questions. If you'd like to propose an enhancement to the repository, feel free to open up a ticket and discuss your improvements before coding a solution. |
3 participants
This PR updates the “Adding a New Worker” documentation by documenting additional required update locations such as worker registration and CI deployment configuration, as referenced in the repository guide.
Closes #582