ethjs / ethjs-contract

A simple contract object for the Ethereum RPC layer.
MIT License
20 stars 28 forks source link

don't pass empty `newMethodCallback` function to ethjs-query call #18

Closed mikec closed 6 years ago

mikec commented 6 years ago

This change to ethjs-query https://github.com/ethjs/ethjs-query/commit/dd1d1b29dc80a31da86704ae840ebdd4acf0e812 will not return a promise if any callback function is passed. This breaks promise returns for .new(), since this was always passing a function through to the ethjs-query sendTransaction call. With this commit, the callback function will only be passed to sendTransaction if a callback was defined.

ethjs-contract

Thank you for contributing! Please take a moment to review our contributing guidelines to make the process easy and effective for everyone involved.

Please open an issue before embarking on any significant pull request, especially those that add a new library or change existing tests, otherwise you risk spending a lot of time working on something that might not end up being merged into the project.

Before opening a pull request, please ensure:

Be kind to code reviewers, please try to keep pull requests as small and focused as possible :)

IMPORTANT: By submitting a patch, you agree to allow the project owners to license your work under the terms of the MIT License.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 5bdc0d64283a5408684ff5e4c4fe591547c7c825 on mikec:fix-promises into 7a75bfce6c0c9bbd17e437d8f6c825f7c96df241 on ethjs:master.

SilentCicero commented 6 years ago

Thanks! Will review today.

Sent from my iPhone

On Apr 9, 2018, at 4:39 AM, Coveralls notifications@github.com wrote:

Coverage remained the same at 100.0% when pulling 71a6b29 on mikec:fix-promises into 7a75bfc on ethjs:master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero commented 6 years ago

Will review tomorrow.

Sent from my iPhone

On Apr 9, 2018, at 4:39 AM, Coveralls notifications@github.com wrote:

Coverage remained the same at 100.0% when pulling 71a6b29 on mikec:fix-promises into 7a75bfc on ethjs:master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero commented 6 years ago

This and other things have been addressed in 2.0 @mikec thanks for the PR regardless!