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 a second scope for i18n enum to follow the syntax in Rails guides #1283

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nicolas-brousse
Copy link

In Rails guides there is the two following paragraph about I18n and ActiveRecord:

The guides talk about this

en:
  activerecord:
    attributes:
      user/gender:
        female: "Female"
        male: "Male"

Instead of

en:
  activerecord:
    attributes:
      user:
        genders:
          female: "Female"
          male: "Male"

Since this I18n scope is present since 5 years ago, my PR add a second scope instead of changing it. That way we are able to use both, and not broke existing Rails applications.

I've to add tests. I just start this as a discussion before continuing that way.

Add a second scope for i18n enum to follow the syntax in Rails guides
@justinfrench
Copy link
Member

Hi @nicolas-brousse, sorry for not reviewing this sooner. This looks sensible, but would need tests before it's merged. Are you in a position to add these (or at least try)?

@nicolas-brousse
Copy link
Author

Hey @justinfrench, no worries, I know that it a quite sensible change. I tried to found an other way to manage it, but didn't found. So I propose it this way.

So yes, if this change sounds interesting for you, I could try to found time to add some tests.
I already tried it temporarily in a real project. But since it's a sensible change, I prefer to work on a test.

I'll try to found some time on next week since I'm currently sick. I'll keep you in touch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants