-
Notifications
You must be signed in to change notification settings - Fork 7
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
add examples for sep-10 #162
Conversation
@@ -0,0 +1,56 @@ | |||
/* eslint-disable */ | |||
|
|||
// @ts-expect-error: express will need to be installed to run this example |
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 thought it was maybe overkill to add express
to the repo just for this example, so I just added this as a comment. Let me know if it's worth adding it so a user can actually spin this up from this repo
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.
Is it too heavy? I'm personally fine adding it as a dev dependency as we could use it for other examples as well, but also OK in case you think that's too much
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 guess it won't affect the bundle size of the library, either. I'll add this as a dev dep 👍
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.
LGTM - left a few comments
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
@@ -69,6 +70,7 @@ | |||
"build:web": "webpack --config webpack.config.js", | |||
"build:node": "webpack --env NODE=true --config webpack.config.js", | |||
"build": "run-p build:web build:node", | |||
"example:sep10": "ts-node examples/sep10/sep10Server.ts", |
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.
how should we runsep10Wallet
?
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.
Ah good call. I wasn't sure if this was useful to run as it's a simple helper, but I can add it!
No description provided.