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

improve clarity of upper case characters and code reuse #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bendmorris
Copy link

Upper case letters can be derived from the lower case ones with only a minor runtime cost. Also added some helpful documentation for people unfamiliar with the concept of letters.

Context: https://twitter.com/bendmorris/status/1438369582125699072

Copy link

@blake2573 blake2573 left a comment

Choose a reason for hiding this comment

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

It would be good to see this merged soon - I've been dealing with risky packages in production while waiting for this massive code reliability improvement to go through.

LGTM!

Copy link

@Samuel-BlankAmber Samuel-BlankAmber left a comment

Choose a reason for hiding this comment

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

Fantastic work. Love to see such a great addition being contributed to my favorite package(s). One minor point of feedback that could help increase widespread adoption and understanding of the project.

export default "J";
/**
* @link https://en.wikipedia.org/wiki/J
* I bet you didn't read through all of these

Choose a reason for hiding this comment

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

Suggested change
* I bet you didn't read through all of these
* I bet you did not read through all of these

As this is in the documentation for the uppercase character J, I suspect many individuals reading this will not be familiar with advanced concepts such as Punctuation and Apostrophes. To ensure the documentation remains readable to beginners of the alphabet, I would encourage you to remove this use of an Apostrophe.

@nbitzz
Copy link

nbitzz commented Jan 3, 2025

Hi, do we have an ETA on when this is getting merged? My team would greatly benefit from the superior DX provided by this PR and are eagerly awaiting its merge. Thanks.

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.

4 participants