Use context.locale instead of ::I18n.locale

[docelic]

This commit makes the locale choice thread-safe.

The corresponding changes to the citrine-i18n shard have already been made.

Fixes #776.

[docelic]

@faustinoaq HTTP::Server::Context is expanded with property "locale" inside citrine-i18n shard. But it appears that inside Amber in the place where test fails, "locale" isn't seen. Got any suggestions how to solve it?

(One possible solution would be to extent Context with "locale" inside Amber, and not inside citrine-i18n. Do we do that or there are some other ideas?)

[faustinoaq]

@docelic I think you need to delegate :locale to context here:

amber/src/amber/controller/base.cr

Line 40 in f84fe8c

to: context

BTW, I just realize test error are being printed twice, Can we fix that?

amber/spec/build_spec.cr

Lines 54 to 67 in f84fe8c

	it "can be executed" do
	puts spec_result unless spec_result.includes? "Finished in"
	spec_result.should contain "Finished in"
	end
	it "has no errors" do
	puts spec_result if spec_result.includes? "Error in line"
	spec_result.should_not contain "Error in line"
	end
	it "has no failures" do
	puts spec_result if spec_result.includes? "Failures"
	spec_result.should_not contain "Failures"
	end

We should puts spec_results once IMO 😅 /cc @eliasjpr

Something like this:

puts spec_result # => We can use just one puts here because spec_results output without errors are minimal

it "can be executed" do
    spec_result.should contain "Finished in"
end

it "has no errors" do
    spec_result.should_not contain "Error in line"
end

it "has no failures" do
    spec_result.should_not contain "Failures"
end

[docelic]

@faustinoaq Hm, if I am understanding the error correctly, the problem is not that "locale" doesn't exist, but rather that "context.locale" doesn't exist, which means that somehow Amber::Controller::Helpers::I18n's methods t() and l() were checked before citrine-i18n was required and before it added the property "locale" to Context.

[docelic]

Looks like the update to citrine-i18n v0.2.0 solved the issue, and the current error reported by travis is not directly related to the code.

Could someone check/confirm that's the case? Thanks!

[faustinoaq]

Outdated shard.lock (wrong resolver). Please run shards update instead.

Why? Aren't CI tests created from scratch? 😅

Ref: https://travis-ci.org/amberframework/amber/jobs/382976615

[docelic]

Do we have an update here? Thanks!

[faustinoaq]

Installing citrine-i18n (0.1.0 at 0.2.0)

@docelic Travis is still failing https://travis-ci.org/amberframework/amber/jobs/382976615#L731

You forgot to update 0.1.0 to 0.2.0 on https://github.com/amberframework/citrine-i18n/blob/master/shard.yml#L3 😅

[faustinoaq]

Fixed! https://github.com/amberframework/citrine-i18n/blob/master/shard.yml#L3 🎉

[docelic]

@faustinoaq Oh great, thanks. I indeed forgot. I updated Amber's shard.yml to use the new version :) Thanks!

[faustinoaq]

@docelic BTW, I already fixing recipes indentation based on your comment: amberframework/liquid.cr#31 (comment) 👍

Also, WDYT about moving src/locale to config/locate like rails ? 😅

[docelic]

It was originally @eliasjpr's suggestion that we put it in src/locale. Let's see his current thoughts on that.

[faustinoaq]

Just waiting @eliasjpr opinion about moving src/locales to config/locales like rails 👍