Skip to content
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

Enhance/add alphabet param #40

Merged
merged 12 commits into from
Aug 23, 2019
Merged

Conversation

dannywillems
Copy link
Contributor

This PR adds the possibility to specify different alphabet in the functions to include, for example, the ripple base58 alphabet: https://developers.ripple.com/base58-encodings.html

I also renamed for more clarity the alphabet module variable in BITCOIN_ALPHABET, and add the ripple alphabet too.

I suggest to update the version to 1.1.0 as it could break if people uses base58.alphabet

@keis
Copy link
Owner

keis commented Feb 25, 2019

Thanks for the PR @dannywillems

A few minor nitpicks that it would be great if you could fix but otherwise looks good.

  • Please squash the pep8 fixes with the commit that introduce the issue and either complete the docstyle change or leave it be.
  • Instead of making a breaking change could you add alphabet as a alias for BITCOIN_ALPHABET?
  • Update the examples in the README to show how to use the alphabet flag

@keis keis merged commit a7d983a into keis:master Aug 23, 2019
@keis
Copy link
Owner

keis commented Aug 23, 2019

Thanks!

(Sorry for not getting around to this earlier for some reason github doesn't notify on code updates to PRs ...)

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.

None yet

2 participants