artsy / README

:wave: - The documentation for being an Artsy Engineer
Creative Commons Attribution 4.0 International
1.1k stars 120 forks source link

RFC: Incrementally adopt I18n library in Rails projects #421

Closed lidimayra closed 2 years ago

lidimayra commented 3 years ago

Proposal:

Incrementally adopt I18n library in Rails projects

What is i18n?

I18n is the standard Rails library for internationalization and localization. What do those terms mean?

The process of "internationalization" usually means to abstract all strings and other locale specific bits (such as date or currency formats) out of your application. The process of "localization" means to provide translations and localized formats for these bits

(from RailsGuides)

Adoption at Artsy

Reasoning

Exceptions:

None?

Additional Context:

Rails Guides on i18n W3C standards on Internationalization

How is this RFC resolved?

We communicate the outcome to the wider Product team via standups, emails, and Slacks. We all become responsible for enforcing the outcome in PR reviews, project planning, and our own code.

kajatiger commented 3 years ago

Yeeey I talked about something related yesterday with @ansor4 and would be super happy to be one step further into having a globalized codebase that dynamically generates localized views but keeps the data as neutral and local-agnostic as possible.

damassi commented 3 years ago

I think about this topic all the time, and what it would mean for projects like Force 👍 I think we'd have to do a solid business analysis on the need before undertaking such a large engineering task but yeah, as we grow this is an important topic.

damassi commented 3 years ago

As a side note, Ohm set something like this up a while back: https://github.com/artsy/ohm/blob/master/config/locales/en.yml

lidimayra commented 2 years ago

Yeah, I see we already have it on many projects (like Gravity, Pulse, Exchange), so it would be just a matter of starting to use those.

For React projects like Force, I guess we could also use something like format-js. I just didn't want to suggest it in the same RFC because I would think the choice of the library might lead to a parallel discussion.

jonallured commented 2 years ago

Interesting! Does this RFC make any distinctions between admin-only apps and client-facing apps? Like, we wouldn't bother with i18n for an app that's only meant to be used by internal Artsy staff right? Or does this RFC really mean ALL Rails apps at Artsy? 🤔

lidimayra commented 2 years ago

Yeah, I wasn't thinking about any kind of distinction... from my perspective, I think that it's easier to maintain software when those layers are separated (text content from application logic). So yes, I was actually thinking about an incremental adoption taking all Rails apps into account. But I wonder if this approach could have cons that I'm not thinking about, it would be great to hear more on that!

anandaroop commented 2 years ago

I agree that this is something worth considering for customer-facing apps, which is why Volt has always been pretty good about following the Rails i18n conventions — though it's worth noting that in its lifetime of 7+ years we've never actually created a localization other than US English.

I'm less convinced that internal admin apps need this overhead, so long as English remains the default lingua franca at Artsy offices. (I do feel the overhead and friction are real, as finding the right locale string to change in Volt, and keeping the locale files organized in a sane way can require some effort.)

As you suggest in the description, a separate and maybe bigger discussion is how to handle this on the React side. As far as I'm aware even Volt does not yet have a good answer for this (and of course Force has never even attempted this).

Rails+React+i18n was not a well-solved problem last I checked but I haven't kept up to date on this. I think this might be fertile ground for some Future Friday / hackathon type experimentation! But I feel hesitant to endorse this as a recommendation for all our apps.

lidimayra commented 2 years ago

Great inputs, thanks for sharing!! In that case, I would be more than happy to think about the customer-facing apps as a start (and then we could have this discussion again in the future regarding internal admin apps, once we're more used to it).

And I love the idea of the Future Friday / hackathon experimentation in the case of React apps! ❤️

mdole commented 2 years ago

Thanks for raising this idea @lidimayra! Agreed with previous commenters that focusing on customer-facing apps is the right way to go.

I also like the idea of a Future Friday "epic" of sorts for incremental adoption and improvement of internationalization. Could be added to this Notion table, or perhaps even created in Jira to make it easier to track over time.

I do feel the overhead and friction are real, as finding the right locale string to change in Volt, and keeping the locale files organized in a sane way can require some effort.

This is also something I've felt in the past. When I first joined the Engineering team, I found these files extremely confusing - why is the text for the thing I'm working on located in some file halfway across the codebase? And it's one of those ruby "magic" things that only makes sense once you're used to it. I couldn't see an explicit connection between the thing I was working on and the file where the text was stored, so there was a fair amount of head-scratching before I understood it.

@lidimayra Do you have any suggestions for how we could avoid a big tangle of localization files? Are there any public projects we could use as examples that are large but have maintained well-organized text files?

lidimayra commented 2 years ago

Of course!! I added this one to our Future Friday table. I would think we can have one for the user-facing Rails apps to get started, and then if we feel that we're comfortable with the idea of i18n in a broad way, we can register the plan to move forward (handling react apps, for example)

I could register the ticket in Jira as well, but I was a bit confused if it would make sense to have something like this on the backlog/board of some specific team. Would it?

When it comes to i18n practices, I would say that sticking to the same conventions that are already present in the whole project would be the way to go. Using the same example contained in the description, but in detail:

If we look at order_fullfilled_buyer on pulse, we have something like this:

<% content_for :header_preview do %>
  Your order has shipped
<% end %>
<% content_for :header_title do %>
  Artsy Order Shipped
<% end %>
<% content_for :title do %>
  Your order has shipped
<% end %>

With I18n, I would expect to move these strings to be in a yml file located at locales/views/commerce_mailer.en.yml

en:
  commerce_mailer:
    order_fullfilled_buyer:
      your_order_has_shipped: Your order has shipped
      artsy_order_shipped: Artsy Order Shipped

And then, because this file would be following the exact same structure of the project, we wouldn't have to worry about calling the entire locale key path, we should be able to use Rails lazy lookup and have the following:

<% content_for :header_preview do %>
  t('.your_order_has_shipped')
<% end %>
<% content_for :header_title do %>
  t('.artsy_order_has_shipped')
<% end %>
<% content_for :title do %>
  t('.your_order_has_shipped')
<% end %>

We would have exceptions, as in scenarios in which we would want to use the same string in multiple files, then we would want a shared key (untied from specific controllers/views). In these cases, I would think of it as a matter of thinking about the more restricted directory/namespace that it fits.

For example, if the same string is used on an email template that's contained on commerce_mailer/order_fullfilled_buyer and also on commerce_mailer/shipping_reminder_seller (and also on other templates in commerce_mailer), we might consider having:

en:
  commerce_mailer:
    order_fullfilled_buyer:
      your_order_has_shipped: Your order has shipped
      artsy_order_shipped: Artsy Order Shipped
    shared:
      my_shared_string: xpto

And then the difference would be that instead of calling t(.my_shared_string), we would call t('commerce_mailer.shared.my_shared_string'). The path would still be easy to spot for anyone who would have to maintain this.

About large open source projects that currently apply good i18n practices: I'll start an investigation on that.

lidimayra commented 2 years ago

Resolution

We decided that we want to have internationalized implementation in our Rails projects

Level of Support

3: Majority acceptance, with conflicting feedback.

Additional Context:

Starting with user-facing apps makes more sense at this point, so we can avoid unnecessary friction in apps that are meant to be used only internally.

Next Steps

A Future Friday project was registered as a starting point for the implementation.

Exceptions