-
Notifications
You must be signed in to change notification settings - Fork 22
initial-sdk #1
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
initial-sdk #1
Conversation
Signed-off-by: Antonio Mendoza Pérez <antonio.mendoza@bideaavant.com>
README.md
Outdated
|
|
||
| ### Add as dependency to your project | ||
| ```sh | ||
| npm install sdk-typescript |
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 am assuming here that we are going to publish the package into npm (sdk-typescript or @serverlessworkflow/sdk-typescript, but this is not working yet.
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.
agreed, but let's leave this out until we deploy it there
+1 for @serverlessworkflow/sdk-typescript
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.
makes sense,
is it ok if I add a description on how to use the sdk as a dependency with npm-link?
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.
sure! thanks
| To build the project and run tests locally: | ||
|
|
||
| ``` | ||
| git clone https://github.com/serverlessworkflow/sdk-typescript.git |
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.
can you add second step of
"cd sdk-typescript" before the npm install...i know its obvious but just in case :)
README.md
Outdated
| # Serverless Workflow Specification - Typescript SDK | ||
|
|
||
| Provides (TODO: add specifics) for the [Serverless Workflow Specification](https://github.com/serverlessworkflow/specification) | ||
| Provides the Java API/SPI for the [Serverless Workflow Specification](https://github.com/serverlessworkflow/specification) |
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.
Could we maybe indicate that this is still WIP in the readme?
Per your comment it seems there is still some work to be completed at a later point in time?
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.
Yes it is still work to do,
There is a WIP two lines below. Do you want me to be more explicit? I am ok doing it, just want to be sure.
- replace Java for Typescript
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.
Opps I missed that, sry. Yeah this is fine the way you have it.
|
@antmendoza this is really cool! thank you very much. |
Signed-off-by: Antonio Mendoza Pérez <antonio.mendoza@bideaavant.com>
|
@tsurdilo I have updated the readme.md according to your comments, any issue just let me know. Meanwhile I will continue adding functionality to cover the specification. |
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Initial implementation for typescript sdk
Based in the specification v0.6. Only a few subset of definitions, I will continue working on it.
Special notes for reviewers:
Additional information (if needed):