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

Caching #122

Merged
merged 11 commits into from
Oct 25, 2021
Merged

Caching #122

merged 11 commits into from
Oct 25, 2021

Conversation

westonganger
Copy link
Contributor

Continues the work from PR #110

The tests are not all passing yet

Copy link
Collaborator

@timfjord timfjord left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @westonganger
I've added some comments

Rakefile Outdated Show resolved Hide resolved
lib/i18n/backend/active_record.rb Show resolved Hide resolved
lib/i18n/backend/active_record.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/support/minitest_context.rb Outdated Show resolved Hide resolved
@westonganger
Copy link
Contributor Author

Ok now mostly all tests are passing. There are only 3 tests failing on test/missing_cached_test.rb due to the issues stated on #106

lib/i18n/backend/active_record.rb Outdated Show resolved Hide resolved
test/active_record_cached_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/active_record_test.rb Outdated Show resolved Hide resolved
test/missing_test.rb Outdated Show resolved Hide resolved
test/missing_test.rb Outdated Show resolved Hide resolved
@timfjord
Copy link
Collaborator

timfjord commented Oct 25, 2021

@westonganger I was thinking about the caching tests and I would like to handle them a bit differently
Please take a look westonganger#1
Locally it was green, so should be good to go

Please merge the PR so I can merge this PR afterwards as it looks good to me(except the tests part)

Use another approach for caching tests
@timfjord timfjord merged commit 772506c into svenfuchs:master Oct 25, 2021
@timfjord
Copy link
Collaborator

@westonganger Thanks for your hard work on this
This change will be released as 0.5.0 but I am going to do some cleanup and fix the CI first

@dvkch
Copy link
Contributor

dvkch commented Oct 25, 2021

Impressive work @westonganger be @timfjord ! Thank you for continuing and finish this change :)

ddarren pushed a commit to visit-widget/i18n-active_record that referenced this pull request Dec 28, 2021
Co-authored-by: Stanislas Chevallier <[email protected]>
Co-authored-by: Tim Masliuchenko <[email protected]>
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.

3 participants