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 frozen_string_literal: true comments and deprecate default_locale config accessor in favor of locale #89

Merged
merged 4 commits into from
May 13, 2024
Merged

Add frozen_string_literal: true comments and deprecate default_locale config accessor in favor of locale #89

merged 4 commits into from
May 13, 2024

Conversation

ColinDKelley
Copy link
Contributor

This PR

  • Adds frozen_string_literal: true magic comment to all Ruby files.
  • Removes the .freeze from string literals since that now happens automatically with the above comment.
  • Fixes a bug where the :pt locale was pluralizing the shared "'lhão'" string literal (with gsub!) inside the LOTS constant.
  • Deprecates default_locale config accessor in favor of simply locale. (For compatibility the old accessors are still supported by alias.)
  • Updates README for the above.

Motivation for the deprecation: It was inconsistent that the locale config had a default_ prefix, but decimals_as didn't. The README referred the them both as defaults, which makes sense...that's how configs usually work. So I figured the easy way to make them consistent was to drop the default_ prefix. Alternatively, we could add a default_ prefix to decimals_as. Or if it seems controversial, we could just drop the commit from this PR and consider it separately.

@radar
Copy link
Owner

radar commented May 12, 2024

Let's remove that default_locale change from this PR, as I don't want to break anyone's code that's using this option.

@ColinDKelley
Copy link
Contributor Author

Let's remove that default_locale change from this PR, as I don't want to break anyone's code that's using this option.

That commit doesn't break any code, because it has aliases for the previous names default_locale and default_locale=, along with specs. Still, I can remove it if you don't want it.

@radar
Copy link
Owner

radar commented May 13, 2024

Oops, sorry that I missed that alias! Ok. I think it's fine to keep in.

@radar radar merged commit 95ef604 into radar:master May 13, 2024
3 checks passed
@ColinDKelley ColinDKelley deleted the frozen_string_literal-and-locale-rename branch May 13, 2024 03:49
@ColinDKelley ColinDKelley changed the title Add frozen_string_literal: true commends and deprecate default_locale config accessor in favor of locale Add frozen_string_literal: true comments and deprecate default_locale config accessor in favor of locale May 13, 2024
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.

2 participants