Sparrow dom/tranaction hash

OriginProtocol / origin-js

We've moved to a monorepo: https://github.com/OriginProtocol/origin

MIT License

81 stars 33 forks source link

Sparrow dom/tranaction hash #546

Closed sparrowDom closed 5 years ago

sparrowDom commented 5 years ago

Checklist:

[x] Code contains relevant tests for the problem you are solving
[x] Ensure all new and existing tests pass
[x] Update any relevant READMEs and docs

Description:

Connected PR: https://github.com/OriginProtocol/origin-dapp/pull/577

franckc commented 5 years ago

Thanks @sparrowDom for working on this ! I can review once the discussion regarding approach to take on DApp side is settled a bit more... regardinhttps://github.com/OriginProtocol/origin-dapp/pull/577

sparrowDom commented 5 years ago

@franckc yup I guess this is currently more in a suggestion/proof of concept stage.

sparrowDom commented 5 years ago

@tyleryasaka agreed, have created an issue for unification of the api: https://github.com/OriginProtocol/origin-js/issues/555

sparrowDom commented 5 years ago

@tyleryasaka can you please take another look. Have added a more general options approach that could be used in unification api step that we will do some time later.

sparrowDom commented 5 years ago

@franckc thanks for reminding me about tests. Inline comments are addressed and a test added.

sparrowDom commented 5 years ago

@franckc responded to comments and improved the test as you have proposed.

And please don't hold back on the Pull Request comments. Each comment results either in clearing up confusion, improving the code base and/or improving coding skills. For that reason I prefer as many comments as possible.