Fix base32 unicode alphabet#2380
Conversation
|
Hi, @C85297 , This is my first time merging a commit to an opensource codebase, so I am a little nervous, eventhough I have been working as a fullstack developer for past 3 years, this feels like an "uncharted territory". I had read Contributing.md doc and Wiki pages, and tried to make everything as per the docs, but if I have missed something in any case, kindle feel free to enlighten me, I will definitely fix it. The same goes for the given fix, I took help from AI to fix this, but I have both verified the code and tested the logic manually for different cases and it looks good. Would love to hear from you :) . Thank you for the opportunity! |
C85297
left a comment
There was a problem hiding this comment.
This is fantastic, thanks @loki1205 ! Really appreciate the added tests.
I just want to raise that this issue continues to exist within From Base32, and the other base(n) encoders/decoders, in case you want to take a stab at any of them within the same pull request.
Either way, this pull request fixes the issue, so happy for it to be merged as-is.
Also, thank you for the AI disclosure, which is really appreciated. Unfortunately, we are having to perform some internal work to update our policies and Contributor License Agreement before we can merge external AI-contributions. Please do not take this as a reflection of your work, its merely our internal bureaucracy. I hope this will be unblocked soon so that we can merge this pull request.
|
Thanks @C85297 , I am glad that you approved this. And, I understand AI policy, I also saw other PRs in the repo which were not merged. Also, I saw the issue in From Base 32 and other base n code and I left them because I thought to fix only one bug per PR. It is easy to review and maintain. Hence, It would be great if you can raise a bug for the other scenarios. Now, I can do it without AI because I have understood the fix. :) Thanks again! My first contribution was a success 🥳 edit: there are some checks failing (Lint), can I continue to fix that or do I need to wait? edit again: fixed lint issues, and ran npm run lint along with npm run test. Both of them are passing now. So, it should do in PR also 🤞 |
Description
Provide a description of the pull request and the changes that it makes.
Existing Issue
#2355
Screenshots

AI disclosure
AI Tools used:
Regardless of AI tool usage, I take full ownership of the code. This is my first open source bug fix and I might have made some mistakes, if so, kindly feel free to add comment, I will be very happy to fix the comment. But, I fully understand the logic.
Test Coverage
Done, I found 3 files with the usage of the method, I have updated tests in those files.