QN mappings review & fixes #4856
Conversation
There was a problem hiding this comment.
This is a great improvement, and code looks so much more readable.
Small change and merge conflict fix needed otherwise good to go.
I ran processor against mainnet works great. I think it would be nice to have a tool to compare state between two query-node instances.
| }, | ||
| "devDependencies": { | ||
| "eslint-plugin-local-rules": "2.0.0", |
There was a problem hiding this comment.
Adding this package seems to require native build on mac/arm64 so I had to add node-gyp as a dev dependency to successfully build docker image of query-node.
There was a problem hiding this comment.
Addressed in b3f01a3
query-node/mappings/src/common.ts
Outdated
|
|
||
| // log error | ||
| logger.error(errorMessage, data) | ||
|
|
||
| // eslint-disable-next-line local-rules/no-throw | ||
| throw errorMessage |
There was a problem hiding this comment.
Would it make sense to call process.exit here instead of throwing?
This would be to prevent any code capturing this throw, and also caller doesn't need to return from the calling function.
There was a problem hiding this comment.
Addressed in 810240d
|
@zeeshanakram3 I fixed conflicts and updated version here zeeshanakram3#7 |
zeeshanakram3
force-pushed
the
qn_mappings_review
branch
from
ed6913c
to
810240d
Compare
fix merge conflicts to master branch
addresses #4842
This PR:
LeadasContentActor#4855UpdateGlobalNftLimitproposal type #4853throwkeywordthrowfrom the mappings and adds helper functiongetById&getByIdOrFailinstead