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

Make pluralSpecs non-global #82

Closed
bep opened this issue Nov 2, 2017 · 3 comments · Fixed by #92
Closed

Make pluralSpecs non-global #82

bep opened this issue Nov 2, 2017 · 3 comments · Fixed by #92
Assignees
Milestone

Comments

@bep
Copy link
Contributor

bep commented Nov 2, 2017

This replaces the rejected #81

While @nicksnyder is right about thread safety being the client's responsibility, the global pluralSpecs makes it harder than it should.

And then having RegisterPluralSpec mutate that state.

In Hugo we run most tests with t.Parallel(). And while this may report some data races we may not realistically see in real use scenarios, it is well worth it.

In this case, it reports a data race in pluralSpecs when using the RegisterPluralSpec. We can probably fix that by adding some global locks.

But these globals also has another unfortunate side-effect:

We cannot run multiple go-i18n configurations side-by-side. And this has nothing to do with concurrency.

So, it would be much better if you could provide a constructor func that takes the options needed (plural specs etc.)

@nicksnyder
Copy link
Owner

I agree that global state is not ideal and regret having a global default bundle as well.

bep added a commit to bep/go-i18n that referenced this issue Nov 5, 2017
This is a follow up to this long-lived Hugo issue:

gohugoio/hugo#3564

This may be improved on in nicksnyder#82 -- but this one is needed just go get it done.

The plan for Hugo's part is:

For all the language codes in use:

1. Check if it is already registered (`GetPluralSpec`)
2. If not, register it as an alias to `GetPluralSpec("en")`
@bep bep mentioned this issue Nov 5, 2017
nicksnyder pushed a commit that referenced this issue Nov 5, 2017
This is a follow up to this long-lived Hugo issue:

gohugoio/hugo#3564

This may be improved on in #82 -- but this one is needed just go get it done.

The plan for Hugo's part is:

For all the language codes in use:

1. Check if it is already registered (`GetPluralSpec`)
2. If not, register it as an alias to `GetPluralSpec("en")`
@nicksnyder nicksnyder added this to the v2 milestone Feb 14, 2018
@nicksnyder
Copy link
Owner

V2 contains a proposal to solve this. Please review #92

@nicksnyder nicksnyder mentioned this issue Apr 10, 2018
Merged
4 tasks
@nicksnyder nicksnyder self-assigned this Apr 10, 2018
@nicksnyder
Copy link
Owner

I just tagged 2.0.0.beta.1. Please start using it and report any issues that you have.

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 a pull request may close this issue.

2 participants