add locale.rake #717

JasonYCHuang · 2017-02-15T14:07:22Z

Solve travis CI problem in shakacode/react-webpack-rails-tutorial#340

This change is

coveralls · 2017-02-15T14:19:48Z

Coverage remained the same at 99.329% when pulling 1cfc293 on JasonYCHuang:rake-locale into 2b8ee6a on shakacode:master.

coveralls · 2017-02-15T15:26:28Z

Coverage remained the same at 99.329% when pulling b3b70ce on JasonYCHuang:rake-locale into 2b8ee6a on shakacode:master.

justin808 · 2017-02-16T09:17:02Z

@JasonYCHuang This looks fine. If we have this, then it's optional to add the locale building to the application startup. It would be really slick if we checked for the locale files being current at the same place migrations are confirmed on controller requests....

In order to merge this:

Doc updates (i18n file? README?)
Please see Task description containing a space after a period. ruby/rake#7 for the long description on the rake task. I'd put in a link to the i18n doc file in the GitHub repo.
Changelog entry

JasonYCHuang · 2017-02-17T15:51:41Z

@justin808 I will update documents.

I am confused about Travis CI failure. It only fails in Ruby2.3.1.
So far, I didn't find any clue.
Do you have suggestions on this?

justin808 · 2017-02-17T21:48:27Z

Travis.ci is consistently failing for at least one ruby version and I just restart it for just that version.

justin808 · 2017-02-18T20:34:40Z

@JasonYCHuang See my comment above: #717 (comment)

Do you need any help from me? I'd like to wrap this one up this weekend!

JasonYCHuang · 2017-02-18T20:46:35Z

@justin808 Yes, I need some advice.

I am going to update i18n doc.
If we will use rake task, instead of ReactOnRails::LocalesToJs.new in config/application.rb, to trigger generating locale js files, where should I put this rake script.

In package.json, we can use prebuild hook to make sure this happens before all the build, but we have several build scripts, like build:client, build:server. It is not smart to set rake task in all of them. Where should I put the task?

JasonYCHuang · 2017-02-18T21:01:13Z

@justin808 What do you mean: Please see ruby/rake#7 for the long description on the rake task. I'd put in a link to the i18n doc file in the GitHub repo.?

Should I add more information to desc? or only add a link to i18n.md pointing this rake file?

justin808 · 2017-02-18T21:14:54Z

@JasonYCHuang I sent you invites to Screen Hero and Slack. Let's get this locale stuff resolved! Awesome work!

Should I add more information to desc? or only add a link to i18n.md pointing this rake file?
The more docs the better! Remember this is some plain text. I type rake -D and get a good description of this!

Overall:

Running the build task in config/application.rb is a bit awkward for a production deploy, because the assets should have been precompiled.
Yes, I think the npm scripts should be responsible for ensuring that the rake task runs at the appropriate time. Ideally, during development, we'd have a "watch" version of that task. The alternative is to have a middleware check on requests to see if the locales are outdated. Even better would be if we could have webpack ensure this! All that being said, I think the locales files will change only occasionally outside of the main language. Most of that work is done by separate teams that will do this in a batch.

http://www.marcusoft.net/2015/08/pre-and-post-hooks-for-npm-scripting.html#pre-and-post-hooks-for-custom-script

justin808 · 2017-02-18T22:03:48Z

This is where we fix up precompile by default:

https://github.com/shakacode/react_on_rails/blob/master/lib/tasks/assets.rake#L38

And this doc needs to be updated... as well as the new locale one.
https://github.com/shakacode/react_on_rails/blob/master/docs/additional-reading/heroku-deployment.md#more-details-on-precompilation-using-webpack-to-create-javascript-assets

coveralls · 2017-02-20T14:04:45Z

Coverage remained the same at 99.329% when pulling 724eba8 on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-20T14:04:45Z

Coverage remained the same at 99.329% when pulling 724eba8 on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-20T14:04:45Z

Coverage remained the same at 99.329% when pulling 724eba8 on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

justin808 · 2017-02-20T21:22:25Z

lib/tasks/assets.rake

+When "ReactOnRails.configuration.i18n_dir" is set, it indeicates that javascript locale files are needed.
+This task generates javascript locale files: `translations.js` & `default.js`.
+    DESC
+    desc "Generate i18n javascript files"


Formatting is not right See example above in this file.

desc << Generate i18n javascript files This task generates javascript locale files: `translations.js` & `default.js` and places them in the "ReactOnRails.configuration.i18n_dir". DESC

coveralls · 2017-02-21T13:02:49Z

Coverage remained the same at 99.329% when pulling 6f06f7f on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-21T13:02:49Z

Coverage remained the same at 99.329% when pulling 6f06f7f on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-21T13:02:49Z

Coverage remained the same at 99.329% when pulling 6f06f7f on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-21T13:41:59Z

Coverage remained the same at 99.329% when pulling 23162e7 on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

coveralls · 2017-02-21T19:24:38Z

Coverage remained the same at 99.329% when pulling 178cc72 on JasonYCHuang:rake-locale into b20b34d on shakacode:master.

JasonYCHuang · 2017-02-23T08:54:36Z

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

docs/basics/i18n.md, line 58 at r3 (raw file):

Previously, justin808 (Justin Gordon) wrote…

assume is still mispelled

and in the react-webpack-rails-tutorial

you do a prebuild:dev

updated

Comments from Reviewable

coveralls · 2017-02-23T09:07:45Z

Coverage remained the same at 99.329% when pulling 5f502c7 on JasonYCHuang:rake-locale into 3106b4b on shakacode:master.

coveralls · 2017-02-27T13:20:08Z

Coverage remained the same at 99.329% when pulling 27fc9be on JasonYCHuang:rake-locale into 8e47c79 on shakacode:master.

coveralls · 2017-02-27T13:20:08Z

Coverage remained the same at 99.329% when pulling 27fc9be on JasonYCHuang:rake-locale into 8e47c79 on shakacode:master.

coveralls · 2017-02-27T13:32:16Z

Coverage remained the same at 99.329% when pulling cb75a4f on JasonYCHuang:rake-locale into 8e47c79 on shakacode:master.

JasonYCHuang · 2017-02-27T13:34:08Z

No pre-hook needed in package.json

to get js locale files in dev: https://github.com/shakacode/react_on_rails/pull/717/files#diff-51a05a5d3f0a0c62b9452b7596e2a82cR2
to get js locale files in test & production: https://github.com/shakacode/react_on_rails/pull/717/files#diff-52cfe83d7f9f1c952a7995dfb0f99341R53
Since config.i18n_dir will be the key to decide whether to generate locale files or not, I comment it for default. When users need i18n, they need to uncomment it. https://github.com/shakacode/react_on_rails/pull/717/files#diff-391d7a2ed038bee9f49a7e7adefc133eR66

Review status: 1 of 6 files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

justin808 · 2017-02-28T09:03:41Z

Very nice.

For testing, we should have a call to generate the i18n task here:

https://github.com/shakacode/react_on_rails/blob/master/lib/react_on_rails/test_helper/ensure_assets_compiled.rb#L29

Reviewed 1 of 1 files at r5, 3 of 5 files at r6, 2 of 2 files at r7.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

CHANGELOG.md, line 10 at r7 (raw file):

### Added
- Allow using rake task to generate javascript locale files. [#717](https://github.com/shakacode/react_on_rails/pull/717) by [JasonYCHuang](https://github.com/JasonYCHuang).

Allow using rake task to generate javascript locale files. The test helper automatically creates the localization files when needed.

docs/basics/i18n.md, line 49 at r7 (raw file):

4. Javascript locale files must be generated before `yarn build`. 
  
  Once you setup `config.i18n_dir` as in the previous step, react_on_rails will help you to do this.

for testing and for production deployments. For development, you should adjust your Procfiles so that they run bundle exec rake react_on_rails:localebefore running theyarn run build:development` (webpack watch process)

lib/generators/react_on_rails/templates/base/base/Procfile.dev.tt, line 2 at r7 (raw file):

web: rails s -p 3000
client: sh -c 'rm app/assets/webpack/* || true && cd client && bundle exec rake react_on_rails:locale && yarn run build:development'

nice! Did you try the generator?

lib/tasks/assets.rake, line 53 at r7 (raw file):

sh "cd client && `ReactOnRails.configuration.npm_build_production_command`"
    DESC
    task webpack: :locale do

Nice!

Comments from Reviewable

JasonYCHuang · 2017-02-28T16:02:44Z

done

Review status: 4 of 7 files reviewed at latest revision, 4 unresolved discussions.

CHANGELOG.md, line 10 at r7 (raw file):

Previously, justin808 (Justin Gordon) wrote…

Allow using rake task to generate javascript locale files. The test helper automatically creates the localization files when needed.

done

docs/basics/i18n.md, line 49 at r7 (raw file):

Previously, justin808 (Justin Gordon) wrote…

for testing and for production deployments. For development, you should adjust your Procfiles so that they run bundle exec rake react_on_rails:localebefore running theyarn run build:development` (webpack watch process)

done

lib/generators/react_on_rails/templates/base/base/Procfile.dev.tt, line 2 at r7 (raw file):

Previously, justin808 (Justin Gordon) wrote…

nice! Did you try the generator?

yes, I create a new rails project and install react_on_rails. It looks good.

Comments from Reviewable

justin808 · 2017-02-28T19:19:21Z

@JasonYCHuang Please confirm that you manually verified checking the locales files works in the test helper and I'll merge.

Reviewed 4 of 4 files at r8.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/generators/react_on_rails/templates/base/base/Procfile.dev.tt, line 2 at r7 (raw file):

Previously, JasonYCHuang (Jason Huang) wrote…

yes, I create a new rails project and install react_on_rails. It looks good.

fabulous.

lib/react_on_rails/test_helper/ensure_assets_compiled.rb, line 30 at r8 (raw file):

        self.class.has_been_run = true

        ReactOnRails::LocalesToJs.new if ReactOnRails.configuration.i18n_dir.present?

This is awesome.

@robwise are you OK with us not adding a test here: https://github.com/shakacode/react_on_rails/blob/master/spec/react_on_rails/test_helper/ensure_assets_compiled_spec.rb#L8

Comments from Reviewable

robwise · 2017-02-28T20:20:14Z

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

lib/react_on_rails/test_helper/ensure_assets_compiled.rb, line 30 at r8 (raw file):

Previously, justin808 (Justin Gordon) wrote…

This is awesome.

@robwise are you OK with us not adding a test here: https://github.com/shakacode/react_on_rails/blob/master/spec/react_on_rails/test_helper/ensure_assets_compiled_spec.rb#L8

I would definitely write a test if you are introducing new logic

Comments from Reviewable

JasonYCHuang · 2017-02-28T21:53:49Z

Yes, I manually run ReactOnRails::TestHelper::EnsureAssetsCompiled.new.call under rails console. Local files will be generated when config.i18n_dir is set.

Review status: 7 of 8 files reviewed at latest revision, 3 unresolved discussions.

Comments from Reviewable

… time. (#340) Demonstrates use of i18n (internationalization) with React on Rails. Docs: https://github.com/shakacode/react_on_rails/blob/master/docs/basics/i18n.md Relevant PRs from React on Rails: * shakacode/react_on_rails#642 * shakacode/react_on_rails#717

JasonYCHuang force-pushed the rake-locale branch from b3b70ce to 1cfc293 Compare February 15, 2017 15:29

JasonYCHuang force-pushed the rake-locale branch 4 times, most recently from af03d3f to 1cfc293 Compare February 17, 2017 15:50

justin808 mentioned this pull request Feb 18, 2017

update i18n doc for .travis.yml #710

Closed

JasonYCHuang force-pushed the rake-locale branch from 1cfc293 to 724eba8 Compare February 20, 2017 13:52

justin808 reviewed Feb 20, 2017

View reviewed changes

JasonYCHuang force-pushed the rake-locale branch from 724eba8 to 6f06f7f Compare February 21, 2017 12:50

JasonYCHuang force-pushed the rake-locale branch 2 times, most recently from 1fac7bb to 23162e7 Compare February 21, 2017 13:27

JasonYCHuang force-pushed the rake-locale branch from 23162e7 to 178cc72 Compare February 21, 2017 19:11

JasonYCHuang force-pushed the rake-locale branch from e23c721 to 5f502c7 Compare February 23, 2017 08:53

JasonYCHuang force-pushed the rake-locale branch 2 times, most recently from 4537752 to 1132e9d Compare February 27, 2017 13:02

add locale rake task

9ef0558

JasonYCHuang force-pushed the rake-locale branch from 1132e9d to 27fc9be Compare February 27, 2017 13:06

update Procfile.dev.tt & react_on_rails.rb.tt

6a9c55f

JasonYCHuang force-pushed the rake-locale branch from 27fc9be to cb75a4f Compare February 27, 2017 13:19

update docs

94e176e

JasonYCHuang force-pushed the rake-locale branch from cb75a4f to 110c11b Compare February 28, 2017 16:00

For testing, a call to generate i18n locale files

a10e4db

JasonYCHuang force-pushed the rake-locale branch from 110c11b to a10e4db Compare February 28, 2017 16:29

JasonYCHuang force-pushed the rake-locale branch from 7620b95 to a10e4db Compare February 28, 2017 21:48

justin808 merged commit b966f8b into shakacode:master Mar 1, 2017

JasonYCHuang deleted the rake-locale branch March 6, 2017 17:02

JasonYCHuang restored the rake-locale branch March 6, 2017 17:02

justin808 mentioned this pull request Dec 20, 2017

Support for Rails' i18n pluralization #1000

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add locale.rake #717

add locale.rake #717

JasonYCHuang commented Feb 15, 2017 •

edited by justin808

Loading

coveralls commented Feb 15, 2017 •

edited

Loading

coveralls commented Feb 15, 2017 •

edited

Loading

justin808 commented Feb 16, 2017

JasonYCHuang commented Feb 17, 2017

justin808 commented Feb 17, 2017

justin808 commented Feb 18, 2017

JasonYCHuang commented Feb 18, 2017

JasonYCHuang commented Feb 18, 2017

justin808 commented Feb 18, 2017

justin808 commented Feb 18, 2017

coveralls commented Feb 20, 2017 •

edited

Loading

coveralls commented Feb 20, 2017

coveralls commented Feb 20, 2017

justin808 Feb 20, 2017

coveralls commented Feb 21, 2017

coveralls commented Feb 21, 2017

coveralls commented Feb 21, 2017 •

edited

Loading

coveralls commented Feb 21, 2017 •

edited

Loading

coveralls commented Feb 21, 2017 •

edited

Loading

JasonYCHuang commented Feb 23, 2017

coveralls commented Feb 23, 2017 •

edited

Loading

coveralls commented Feb 27, 2017

coveralls commented Feb 27, 2017

coveralls commented Feb 27, 2017 •

edited

Loading

JasonYCHuang commented Feb 27, 2017

justin808 commented Feb 28, 2017

JasonYCHuang commented Feb 28, 2017

justin808 commented Feb 28, 2017

robwise commented Feb 28, 2017

JasonYCHuang commented Feb 28, 2017

add locale.rake #717

add locale.rake #717

Conversation

JasonYCHuang commented Feb 15, 2017 • edited by justin808 Loading

coveralls commented Feb 15, 2017 • edited Loading

coveralls commented Feb 15, 2017 • edited Loading

justin808 commented Feb 16, 2017

JasonYCHuang commented Feb 17, 2017

justin808 commented Feb 17, 2017

justin808 commented Feb 18, 2017

JasonYCHuang commented Feb 18, 2017

JasonYCHuang commented Feb 18, 2017

justin808 commented Feb 18, 2017

justin808 commented Feb 18, 2017

coveralls commented Feb 20, 2017 • edited Loading

coveralls commented Feb 20, 2017

coveralls commented Feb 20, 2017

justin808 Feb 20, 2017

Choose a reason for hiding this comment

coveralls commented Feb 21, 2017

coveralls commented Feb 21, 2017

coveralls commented Feb 21, 2017 • edited Loading

coveralls commented Feb 21, 2017 • edited Loading

coveralls commented Feb 21, 2017 • edited Loading

JasonYCHuang commented Feb 23, 2017

coveralls commented Feb 23, 2017 • edited Loading

coveralls commented Feb 27, 2017

coveralls commented Feb 27, 2017

coveralls commented Feb 27, 2017 • edited Loading

JasonYCHuang commented Feb 27, 2017

justin808 commented Feb 28, 2017

JasonYCHuang commented Feb 28, 2017

justin808 commented Feb 28, 2017

robwise commented Feb 28, 2017

JasonYCHuang commented Feb 28, 2017

JasonYCHuang commented Feb 15, 2017 •

edited by justin808

Loading

coveralls commented Feb 15, 2017 •

edited

Loading

coveralls commented Feb 15, 2017 •

edited

Loading

coveralls commented Feb 20, 2017 •

edited

Loading

coveralls commented Feb 21, 2017 •

edited

Loading

coveralls commented Feb 21, 2017 •

edited

Loading

coveralls commented Feb 21, 2017 •

edited

Loading

coveralls commented Feb 23, 2017 •

edited

Loading

coveralls commented Feb 27, 2017 •

edited

Loading