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

Add "To Base62" and "From Base62" operations #443

Merged
merged 1 commit into from
Dec 18, 2018
Merged

Add "To Base62" and "From Base62" operations #443

merged 1 commit into from
Dec 18, 2018

Conversation

tcode2k16
Copy link
Contributor

@tcode2k16 tcode2k16 commented Dec 17, 2018

Hi,
I added the "To Base62" and "From Base62" operations. These two operations should function the same as base62.io.

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2018

CLA assistant check
All committers have signed the CLA.

@n1474335 n1474335 merged commit 22454ae into gchq:master Dec 18, 2018
@n1474335
Copy link
Member

Thanks very much for the contribution, this is great.

I tidied up a number of things:

  • Make sure you include a descriptive description.
  • You were converting to and from the byteArray format unnecessarily. By simply defining your input or output type as "byteArray", this will all be done behind the scenes for you. If you do it yourself, you may add extraneous conversion steps in more complex recipes.
  • We generally like to expose configuration options to the user where relevant. The alphabet in this case seems like a potentially useful thing for the user to be able to change, so I added an argument for it.

Thanks again, this has now been merged into version 8.14.0.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants