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

New translations update tasks #91

Merged
merged 3 commits into from
Jun 2, 2017
Merged

New translations update tasks #91

merged 3 commits into from
Jun 2, 2017

Conversation

alepore
Copy link
Contributor

@alepore alepore commented May 22, 2017

We have some rake tasks to manage YAML files: getting the latest master en.yml and updating other languages with the missing keys.
The tasks actually doesn't work and there's a quite of text parsing code that is not tested.
I think no one has used them for years.

This PR removes that code and uses the i18n-tasks to accomplish the YAML sync feature.
i18n-tasksseems a good gem, has tests and does a lot of other things we can use in the future.

A similar work was done years ago on spree (spree-contrib/spree_i18n#640) but was never merged.

- The file download was not working
- Download the file under /config/locales
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Besides the weird Bundler issue for business lads against older Solidus versions 👍

Although I think we can be less strict on the i18n-tasks version.

@@ -40,4 +40,5 @@ Gem::Specification.new do |s|
s.add_development_dependency 'rubocop', '>= 0.24.1'
s.add_development_dependency 'rspec-rails', '~> 3.1'
s.add_development_dependency 'simplecov', '~> 0.9'
s.add_development_dependency 'i18n-tasks', '~> 0.9.15'
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure we can remove the lock to the patch version and only lock to the minor version instead.

This gem can replace our custom, old and untested tasks for Yaml files
management.
This is not compatible with older solidus versions but that's not a problem. We
only need this when using the master branch.
Remove the old code of the solidus_i18n:sync task and document the use of
i18n-tasks.
@alepore
Copy link
Contributor Author

alepore commented May 23, 2017

Ok i solved the bundle problem (makes sense to only use i18n-tasks with the master branch) using a less strict version dependency.

@alepore alepore requested a review from jhawthorn May 24, 2017 08:34
@alepore alepore mentioned this pull request May 24, 2017
@tvdeyen tvdeyen merged commit 0ca92d8 into master Jun 2, 2017
@alepore alepore deleted the i18n-tasks branch June 6, 2017 05:49
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.

2 participants