Skip to content

Further Implementation of the EIP-681 spec + Optimizations#11

Open
lucemans wants to merge 30 commits into
brunobar79:masterfrom
lucemans:master
Open

Further Implementation of the EIP-681 spec + Optimizations#11
lucemans wants to merge 30 commits into
brunobar79:masterfrom
lucemans:master

Conversation

@lucemans

@lucemans lucemans commented May 5, 2022

Copy link
Copy Markdown

For starters, thank you very much for the existence of this package.
As a desire to further support more of the features described in EIP-681 in the modern wallet this PR has been made.

Here are some of the changes that this PR includes

  • Introduce typescript aswell as types for input and output.
  • Shrunk Bundle Size from 144kb => 4.4kb
  • Deprecate qs as it does not support multiple parameters with the same name
  • Introduce more documentation
  • Introduce support for custom function names
  • Introduce support for custom (& overlapping) arguments
  • Expand tests to encompas
    • custom function names
    • custom arguments

Things left to do:

  • Update README.md to reflect changes (working on it 😉)

Looking forward to hearing from you!

@brunobar79 brunobar79 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for doing this! Looks great.
Added a few comments and I'm happy to take another look after the readme is updated and the comments addressed.

Comment thread package.json Outdated
Comment thread package.json Outdated
Comment thread rollup.config.js Outdated
Comment thread package.json Outdated
Comment thread dist/index.d.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
Comment thread tests/index.js Outdated
Comment thread package.json Outdated
@lucemans

lucemans commented May 9, 2022

Copy link
Copy Markdown
Author

Sweet! I will get around to them as soon as possible!

@lucemans lucemans marked this pull request as ready for review May 9, 2022 23:16
@lucemans lucemans requested a review from brunobar79 May 13, 2022 14:25
@svemat01

Copy link
Copy Markdown

Looks cool!

@brunobar79 brunobar79 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! added a few more comments

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md Outdated
Comment thread package.json Outdated
Comment thread tests/index.spec.ts Outdated
Comment thread tests/index.spec.ts Outdated
lucemans and others added 7 commits May 23, 2022 08:41
Co-authored-by: Bruno Barbieri <brunobar79@gmail.com>
Co-authored-by: Bruno Barbieri <brunobar79@gmail.com>
Co-authored-by: Bruno Barbieri <brunobar79@gmail.com>
@lucemans lucemans requested a review from brunobar79 May 23, 2022 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants