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

Update some i18n references in guides #32182

Merged
merged 5 commits into from
Mar 9, 2018

Conversation

shioyama
Copy link
Contributor

@shioyama shioyama commented Mar 7, 2018

The Globalize::Backend::Static class no longer exists in the globalize gem, so far as I can tell. I think this section of the guide is way out of date.

@rails-bot
Copy link

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @pixeltrix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

@shioyama
Copy link
Contributor Author

shioyama commented Mar 7, 2018

Summary of changes:

  • Remove reference to Globalize::Backend::Static which no longer exists in globalize gem
  • Remove reference to rails-i18n google group, which is no longer maintained (last post in 2014, moderators no longer active)
  • Remove confusing reference to Globalize3
  • Add section on storing translations, reference top three popular gems for doing this (Globalize, Mobility, Traco) (Disclosure: I'm the author of Mobility.)

I think a lot more could be done to update this guide, just skimming it I found these issues but there are probably more. I think adding a section on translating content is important because it's a topic that comes up a lot, and can be very confusing for people unaware of the distinction with the standard I18n API.

Also: I'm aware that technically you can store strings in the database with i18n using the ActiveRecord backend, but it's an edge use case and gems like Globalize and Mobility are much more commonly used for this.

@shioyama
Copy link
Contributor Author

shioyama commented Mar 7, 2018

@svenfuchs Since you were the original author of this guide, maybe you could comment on the changes? 😄

@shioyama shioyama force-pushed the remove_globalize_static_backend branch from 48f33c8 to b15722c Compare March 7, 2018 01:01
This reference doesn't really make sense here, since Globalize is
primarily focused on storing translated content (now anyway).
@shioyama shioyama force-pushed the remove_globalize_static_backend branch from b15722c to b864169 Compare March 7, 2018 01:08
@shioyama
Copy link
Contributor Author

shioyama commented Mar 7, 2018

There were some formatting issues, fixed them in 5cb14f0.

@pixeltrix pixeltrix merged commit fc55c34 into rails:master Mar 9, 2018
@pixeltrix
Copy link
Contributor

@shioyama thanks!

@shioyama
Copy link
Contributor Author

shioyama commented Mar 9, 2018

Thanks! I don't suppose there's any chance this could get cherry-picked to 5-2-stable as well? Or should I make a PR to that branch for that?

pixeltrix pushed a commit that referenced this pull request Mar 9, 2018
* Remove reference to Globalize::Backend::Static as this class no longer exists.
* Remove reference to google group
* Remove confusing reference to Globalize3
* Add section on translating stored content

[ci skip]
@pixeltrix
Copy link
Contributor

@shioyama cherry-picked to 5-2-stable in 78d47aa

@shioyama
Copy link
Contributor Author

shioyama commented Mar 9, 2018

👍 Thanks!

@svenfuchs
Copy link

Thanks for all your great work in the I18n space, @shioyama!

❤️

@shioyama
Copy link
Contributor Author

shioyama commented Mar 9, 2018

😊

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.

None yet

4 participants