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

Should use uppper case when encoding #6

Open
danny-cheung opened this issue Oct 24, 2019 · 4 comments · May be fixed by #7
Open

Should use uppper case when encoding #6

danny-cheung opened this issue Oct 24, 2019 · 4 comments · May be fixed by #7

Comments

@danny-cheung
Copy link
Contributor

https://en.wikipedia.org/wiki/Base32#Crockford's_Base32 suggests that the encoded string should only use upper case characters.

@danny-cheung
Copy link
Contributor Author

Not sure if this would be a breaking change.

@isoos
Copy link
Contributor

isoos commented Oct 24, 2019

I think that'd be a breaking change now, and for very little benefit. Maybe adding an extra method or a parameter to them?

@danny-cheung
Copy link
Contributor Author

I disagree with the "very little benefit" part. By using lower case, the output is non-compliant and therefore incorrect.

@danny-cheung danny-cheung linked a pull request Oct 24, 2019 that will close this issue
@isoos
Copy link
Contributor

isoos commented Oct 24, 2019

I disagree with the "very little benefit" part. By using lower case, the output is non-compliant and therefore incorrect.

#7 would now break all the codes that rely on the lowercase ids that this code generates. I know at least two apps that use it such way, and their database is case sensitive.

If the code is only soft-compliant, I am willing to live with it.

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 a pull request may close this issue.

2 participants