-
-
Notifications
You must be signed in to change notification settings - Fork 130
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
Version 2.0 #122
Version 2.0 #122
Conversation
Tested while working on solidusio/solidus#2684. Working great so far 👍 and fixes lots of bugs while switching admin locales. We should drop < 2.6 support IMO. There is still a version working for < 2.6 and translations files can easily be updated in the store application (this is recommended anyway) |
README.md
Outdated
|
||
bin/rails g solidus_i18n:install | ||
1. Add gem to your `Gemfile`, then run `bundle install` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this gem to your ...
We should remove |
solidus_i18n.gemspec
Outdated
s.add_runtime_dependency 'solidus_core', ['>= 1.1', '< 3'] | ||
s.add_runtime_dependency 'solidus_support' | ||
s.add_runtime_dependency 'deface', '~> 1.0' | ||
|
||
s.add_development_dependency 'byebug' | ||
s.add_development_dependency 'capybara', '~> 2.17' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still have tests left, then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to remove them still. I'd like some extremely simple tests (basically, verify YAML is valid). We won't need capybara
As far as I can tell engines do this by default and this is unnecessary.
We no longer have any
These will be built-in to Solidus in version 2.6.
We no longer have any files which would be autoloaded by this. This shouldn't really be done anyways (making lib autoloaded, use app).
We no longer have any functionality, so just test that we're adding the locales we want to.
Users can add this themselves
bd08de0
to
3934a3d
Compare
This is ready to go I'm not sure what we should do to support versions < 2.6. Maybe a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what we should do to support versions < 2.6. Maybe a solidus_i18n_backports gem to inject the new Solidus 2.6 functionality into old versions?
I don’t think we need to. Maybe we release a 1.3 version that restricts the version of Solidus to < 2.6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need solidus_support
solidus_i18n.gemspec
Outdated
s.add_runtime_dependency 'i18n_data', '~> 0.7.0' | ||
s.add_runtime_dependency 'rails-i18n', ['>= 4.0.1', '< 6'] | ||
s.add_runtime_dependency 'kaminari-i18n', '~> 0.5.0' | ||
s.add_runtime_dependency 'routing-filter', '~> 0.6.0' | ||
s.add_runtime_dependency 'solidus_core', ['>= 1.1', '< 3'] | ||
s.add_runtime_dependency 'solidus_support' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need solidus_support
? Couldn’t find any usages anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope!
This removes almost all of the functionality from the extension in favour of the new functionality which is built-in to Solidus itself.
This proposed version 2.0 of solidus_i18n provides nothing other than locale files, doesn't require deface, and doesn't do any
class_eval
or module prepends.Functionality moved to Solidus itself:
set_user_language
behaviour solidusio/solidus@568ca0bRemoved functionality:
TODO: