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

Secondary locale lookup #8

Merged
merged 1 commit into from
Feb 17, 2017
Merged

Secondary locale lookup #8

merged 1 commit into from
Feb 17, 2017

Conversation

jcrugzz
Copy link
Contributor

@jcrugzz jcrugzz commented Feb 16, 2017

If the given locale doesn't exist, check the more generic version of the locale to see if a more general build is applicable.

@burgessa23
Copy link

LGTM 👍

Copy link
Contributor

@indexzero indexzero left a comment

Choose a reason for hiding this comment

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

Note about IETF language code Vs BCP47 country code

if (!build && !specs.locale) return notFound();
else if (!build && specs.locale) {
let parts = specs.locale.split('-');
if (parts.length !== 2) return notFound();
Copy link
Contributor

Choose a reason for hiding this comment

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

What about just en? Since we are just using the first part below shouldn't that be valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@indexzero yea but that wouldn't hit this retry case, that would have been what was requested initially. this would handle fallback from en-US to en or es-ES to es

@indexzero
Copy link
Contributor

+1

@indexzero indexzero merged commit 0efb48b into master Feb 17, 2017
@jcrugzz jcrugzz deleted the retry-build-lookup branch February 17, 2017 00:26
@Swaagie
Copy link
Member

Swaagie commented Feb 17, 2017

If we do this now anyways, couldn't we also supply an array of locales and go from [0] to [last] on the array to fetch locales. This solves some of the internal issues we have with reading locales from package.json, checking indexOf against that and then providing the right locale. Would be awesome and eliminate tons of optional code if we can just supply es-ES,es-US,en-US Similarly, browser ship a comma separated string to get a response from the server in a preferred locale.

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