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 label and change default dev locale #13134

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

Conversation

dacook
Copy link
Member

@dacook dacook commented Feb 10, 2025

ℹ️ Funded Feature. Please track ALL ASSOCIATED WORK under the associated tracking code #11678 DFC Orders

What? Why?

I attempted to change a default translation, but wasn't able to see it at first, because the dev environment defaults to en_AU (and I didn't change the en_AU.yml file).
This was confusing, and I thought it would be less confusing to use the same setup that we use for specs.

Now, we probably shouldn't be updating the default translation. We could just update it as needed for each locale.
But in this case, it's a new feature that hasn't been rolled out yet, so I thought it worth updating the default to something more descriptive.

A field label is updated, but it hardly seems worth announcing as a user-facing change. It will be communicated to Garethe (who requested it) anyway.

What should a dev test?

  • Check you don't have anything in .env.development.local that overrides this
  • Boot up a rails server
  • Try changing languages. You should be able to choose English (if you hover over the link you'll see the name "en_TST").
  • Everything appears correct, as per the default translation.

Dependencies

(PR starts at Use test locale instead of real one in development e7054f7)

Makes it clearer where things are going from, and to. I will re-use this order on the next screen.
Making way for a review step.
If there's a matching product in OFN already, a link will appear.
en_AU contains translations for most keys, and is subject to change. So if you change the value of an existing key in en.yml, you won't see the change in your dev environment.

Maybe it's confusing because it's not called 'development', but I think the comment above explains well enough.
This could have been done in Transifex, but I figured it would be quicker to just go ahead and update here.
Because it doesn't fit in db field spree_users.local with limit: 6.
We could extend the field but it seemed silly because it's only used in test/dev environments.
@dacook dacook added the technical changes only These pull requests do not contain user facing changes and are grouped in release notes label Feb 10, 2025
@dacook dacook self-assigned this Feb 10, 2025
@dacook dacook changed the title Fix locale 12301 Update label and change default dev locale Feb 10, 2025
Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Nice 👍

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

👍

.env.development Outdated
Comment on lines 9 to 11
# successful fallback to `en`. You will also see up-to-date text used
# in production
LOCALE="en_AU"
LOCALE="en_TEST"
Copy link
Member

Choose a reason for hiding this comment

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

Part of the comment is not valid anymore:

You will also see up-to-date text used in production

Copy link
Member Author

Choose a reason for hiding this comment

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

True. The downside to this change is that we're not looking at a production locale.
The upside is that it's agnostic of production locales.
🤷

enterprise: "Enterprise"
enterprise: "Create products for enterprise"
Copy link
Member

Choose a reason for hiding this comment

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

Transifex may just override this again.

Copy link
Member Author

Choose a reason for hiding this comment

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

😞

[skip ci]
@dacook dacook added the blocked label Feb 12, 2025
@dacook
Copy link
Member Author

dacook commented Feb 12, 2025

I've already tested as per the above notes. Not sure it's worth getting someone else to test.

But in any case this is blocked until dependent PR is merged.

@mkllnk
Copy link
Member

mkllnk commented Feb 12, 2025

I don't think that we need to test this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Dev Test Ready 🔧
Development

Successfully merging this pull request may close these issues.

3 participants