-
Notifications
You must be signed in to change notification settings - Fork 10
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
Enabled 3rd party imports #46
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.
LGTM
README.md
Outdated
@@ -6,6 +6,7 @@ Up-to-date documentation on Chainlink Functions can be found [here](https://docs | |||
|
|||
# Table of Contents | |||
|
|||
- [functions-toolkit](#functions-toolkit) |
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.
nit: not sure we need this in the TOC given the best it will do is move readers 9 lines up?
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.
Fixed this. Thanks!
'const { format } = await import("npm:date-fns"); return Functions.encodeString(format(new Date(), "yyyy-MM-dd"));', | ||
}) | ||
|
||
expect(Buffer.from(result.responseBytesHexstring?.slice(2) as string, 'hex').length).toEqual( |
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.
should we be testing that the import works as expected by asserting against the value of the result hex versus the length of the hex?
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.
If the import doesn't work, then this whole this will just throw an error, so I don't think this is needed.
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.
@KuphJr sorry wasnt clear- - was suggesting an alternate assert on this existing test
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.
Some minor comments/questions.
SonarQube Quality Gate
|
No description provided.