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

Chore/fix iso locales #14835

Merged
merged 7 commits into from
Nov 24, 2022
Merged

Chore/fix iso locales #14835

merged 7 commits into from
Nov 24, 2022

Conversation

ivanThePleasant
Copy link
Contributor

What does it do?

Removes duplicate ISO locales

Why is it needed?

Duplicate ISO locales break UI of some elements

How to test it?

Run the getstarted app. Watch the video below. The issue in the video should be gone.

https://www.loom.com/share/5c4a5d5683414bbab559c16ecb7a6ef8

Related issue(s)/PR(s)

#14752

@ivanThePleasant ivanThePleasant added source: plugin:i18n Source is plugin/i18n package pr: chore This PR contains chore tasks (cleanups, configs, tooling...) labels Nov 10, 2022
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 49.80% // Head: 49.80% // No change to project coverage 👍

Coverage data is based on head (e9c3b72) compared to base (ab9248d).
Patch has no changes to coverable lines.

❗ Current head e9c3b72 differs from pull request most recent head 9a2046f. Consider uploading reports for the commit 9a2046f to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14835   +/-   ##
=======================================
  Coverage   49.80%   49.80%           
=======================================
  Files         290      290           
  Lines       10184    10184           
  Branches     2252     2252           
=======================================
  Hits         5072     5072           
  Misses       4214     4214           
  Partials      898      898           
Flag Coverage Δ
unit 49.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

I don't understand how duplicates made their way into the file. That's quite concerning 🤔
Would you mind adding a quick unit test that reads the locales file & make sure there are no duplicates in it? This way, we prevent the same issue from happening in the future. WDYT?

@ivanThePleasant
Copy link
Contributor Author

@Convly good point! Done! 😊

@Convly Convly self-requested a review November 15, 2022 09:31
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM

Might be worth it for Derrick to validate the issue has been fixed in his exact scenario.

Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

Lgtm

@ivanThePleasant ivanThePleasant merged commit 00a3f69 into main Nov 24, 2022
@ivanThePleasant ivanThePleasant deleted the chore/fix-iso-locales branch November 24, 2022 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: chore This PR contains chore tasks (cleanups, configs, tooling...) source: plugin:i18n Source is plugin/i18n package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants