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

Switching the default locale to :en #688

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

Conversation

spacekookie
Copy link
Collaborator

Addresses an issue discovered in #681 (related #686)

@ghost ghost assigned spacekookie Jun 25, 2018
@ghost ghost added the in progress label Jun 25, 2018
@spacekookie
Copy link
Collaborator Author

@1000miles Any idea about this failure?

@1000miles
Copy link
Collaborator

1000miles commented Jun 26, 2018

@spacekookie cc @Svenyo

Please keep :de as default locale.

  1. A lot of test cases are based on german locale, if you change it to :en you would need to adjust/fix all the test cases as well. I assume that's why your PR Switching the default locale to :en #688 fails (some of the test failures indicate it).

  2. In the past we also wanted to unify the default locale for the wheelmap project AND transifex but as far as I remember @holgerd once said they want to keep the german locale because of historical reasons, e.g. search engines already have :de indexed as default locale.

  3. If we introduce new keys & translations in english without providing the german translations as well, the system would throw an error message like translation key is missing anywhere where no translations in other locales exist yet.

CORRECTED:

  • We get the keys and translations of them in EN from Svenja.
  • We introduce them into the wheelmap project in the EN resource files.
  • We put the same keys (EN) in the german resource files to prevent the error translation key is missing (MUST BECAUSE DE IS LOCALE IN WHEELMAP).
  • Sometimes we provide the german translation for the introduced keys in the DE resource files as well if they are just 1-3 translations in german (VOLUNTARILY).

My suggestion: Keep it as it is and wait until new translations are coming up for "Datenschutzbestimmungen".

Note: Even though it works at the moment I think for consistency the key privacy should be put to the keys structure for footer instead of header. Here is an example where the structure maps to the translations resource files:

https://github.com/sozialhelden/wheelmap/blob/8df541107ed785f82a2436ebe53d11129e7e2fba/config/locales/en/wheelmap.yml#L351-L353

=> https://github.com/sozialhelden/wheelmap/blob/master/app/views/relaunch/_footer.html.haml#L30

Copy link
Collaborator

@1000miles 1000miles left a comment

Choose a reason for hiding this comment

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

@spacekookie Do you run the tests before pushing your PRs to Github? You could do it via bundle exec rspec spec.

@spacekookie
Copy link
Collaborator Author

@1000miles That would be fine for me. Depends on what @Svenyo thinks.

Also, no I didn't run the tests locally, I was moving from place to place a lot. But…checking tests asynchronously is kinda what failing PR CI is for, isn't it 😉

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

Successfully merging this pull request may close these issues.

None yet

2 participants