-
Notifications
You must be signed in to change notification settings - Fork 8
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
Expand adaptor docs #260
Expand adaptor docs #260
Conversation
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.
hey was the line break on line 65 intentional? please remove if not. (there still be something going on with your formatter and the prettierrc configuration.)
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.
I've made a couple of comments.
It would be really nice to have a monorepo tool to initialise a new adaptor using the template. It could do most of the monkeywork of coping files around and swapping out adaptor names. That would simplify a lot of this stuff!
README.md
Outdated
|
||
The configuration schema defines required parameters and their expected values | ||
- Update the top-level changelog comment `# @openfn/language-template` to your adaptor name and delete the example content. Delete ast.json, then run `pnpm clean` (this will remove the dist, types and docs folders). |
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.
We probably still want the language
- prefix so can we say:
Update the top-level changelog comment `# @openfn/language-template` to `# @openfn/language-youradaptorname`
README.md
Outdated
|
||
#### 2. Create a job with the operation you'd like to add, and run it so it fails | ||
|
||
- Create a _temporary_ folder where you can test your operation manually by running a job: `mkdir tmp` |
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.
I wouldn't emphasis temporary, it makes it look like you have to do something special. Like, how do I create a temporary folder?
Probably important to note that the contents of tmp
don't get checked in to github - so this folder name isn't arbitrary, it's a convention we use.
README.md
Outdated
- The job should fail - we haven't build the adaptor yet! Let's go do that | ||
|
||
#### 3. Create your adaptor function | ||
- Head to `lib/Adaptor.js`, and define your first function. |
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.
src
d2aae67
to
a4cd838
Compare
I am merging this in for today's release. It's an improvement on the current docs and we can raise issues/PRs later if we want to make further changes. Thanks @amberrignell !! |
Summary
Add a high-level, single-sentence summary of what this PR changes.
Details
Add technical details of what you've changed (and why).
Issues
Reference any related issues here.
Review Checklist
Before merging, the reviewer should check the following items:
migration guide followed?
dev only changes don't need a changeset.