-
Notifications
You must be signed in to change notification settings - Fork 272
Conversation
Check won't pass because coverage doesn't upload? |
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. Makes sense not to create the strings if they aren't needed.
I don't know what's going on with the CI. I tried re-running it and it failed in another way.
@evertonfraga could you take a look why Upload to Coveralls is not working in the CI? 😄 |
Sure, sir! |
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.
Done and fixed. Feel free to merge!
The main reason was a command trying to fetch a SHA reference from git, when it didn't exist.
|
Thanks a lot @evertonfraga 😄 |
When implementing Homestead in this PR I ran into an issue when trying to copy SHA3 large buffers which is caused by
assertIsBuffer
. This copies Buffers with sizes which node cannot handle even if there are no errors:When you try to copy a Buffer into a string which is too large, it throws. Besides that, it does not make sense to create these messages if there are no errors (this is unnecessary copying).