-
Notifications
You must be signed in to change notification settings - Fork 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
support custom score api url as function param (and removed unused provider param) #403
Conversation
@bonustrack Removing provider param can be a big change right, It may break for people who already integrated snapshot.js |
@ChaituVR as long as nobody installs it Should then probably be a |
(and remove unnecessary type annotation)
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.
Oops there are conflicts :P
@mktcode This looks good to me, ready to merge this, but the build is failing in GitHub actions with the below error: src/index.ts → dist/snapshot.min.js...
[!] (plugin rpt2) Error: /home/runner/work/snapshot.js/snapshot.js/src/validations/basic/index.ts(19,26): semantic error TS2554: Expected 6 arguments, but got 4.
src/validations/basic/index.ts
Error: /home/runner/work/snapshot.js/snapshot.js/src/validations/basic/index.ts(19,26): semantic error TS2554: Expected 6 arguments, but got 4.
at error (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup/dist/shared/rollup.js:5265:30)
at throwPluginError (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup/dist/shared/rollup.js:17956:12)
at Object.error (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup/dist/shared/rollup.js:18563:24)
at Object.error (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup/dist/shared/rollup.js:18125:38)
at RollupContext.error (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/src/rollupcontext.ts:37:18)
at lodash_3 (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/src/print-diagnostics.ts:41:11)
at arrayEach (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/node_modules/lodash/lodash.js:516:11)
at _.each (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/node_modules/lodash/lodash.js:9342:14)
at printDiagnostics (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/src/print-diagnostics.ts:9:2)
at Object.transform (/home/runner/work/snapshot.js/snapshot.js/node_modules/rollup-plugin-typescript2/src/index.ts:242:5) |
Yeah I saw that... didn't take a closer look yet. Will do today. |
Resolved. I'm not so familiar with TypeScript and the syntax for setting the function param's default values was wrong. |
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 👍
fixes: #402
Needs adjustments in more places once snapshot.js version gets bumped up.