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

Update generator to add ReactOnRails spec helper #362

Merged
merged 1 commit into from
Apr 3, 2016

Conversation

justin808
Copy link
Member

  • Generator adds line to spec/rails_helper.rb so that running specs will
    ensure assets are compiled.
  • Added more diagnostics for server rendering.
  • Calls to setTimeout and setInterval are not logged for server
    rendering unless env TRACE_REACT_ON_RAILS is set to YES.
  • Other small changes to the generator including adding necessary npm
    scripts to allow React on Rails to build assets.
  • npm modules updated for generator.
  • Updated all npm dependencies to latest.
  • Update to node 5.10.0 for CI
  • Change babel-runtime to be a peer dependency.

This change is Reviewable

@justin808 justin808 force-pushed the add-rails-helper-to-generator branch 4 times, most recently from 5d69c70 to fee5e5e Compare April 2, 2016 07:39
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e6a6526 on add-rails-helper-to-generator into * on master*.

@justin808 justin808 force-pushed the add-rails-helper-to-generator branch from e6a6526 to 42301a8 Compare April 2, 2016 10:05
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 42301a8 on add-rails-helper-to-generator into * on master*.

@jbhatab
Copy link
Member

jbhatab commented Apr 3, 2016

lib/generators/react_on_rails/base_generator.rb, line 194 [r2] (raw file):
What about writing these out separately so they stack?


Comments from Reviewable

@jbhatab
Copy link
Member

jbhatab commented Apr 3, 2016

lib/generators/react_on_rails/templates/redux/base/client/app/bundles/HelloWorld/containers/HelloWorld.jsx, line 14 [r2] (raw file):
I thought it was best practice to still use class syntax? I don't care much either way though, they both look good.


Comments from Reviewable

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b7b2ca4 on add-rails-helper-to-generator into * on master*.

@justin808
Copy link
Member Author

Review status: 0 of 19 files reviewed at latest revision, 2 unresolved discussions.


lib/generators/react_on_rails/base_generator.rb, line 194 [r2] (raw file):
Not sure what you mean by this.


lib/generators/react_on_rails/templates/redux/base/client/app/bundles/HelloWorld/containers/HelloWorld.jsx, line 14 [r2] (raw file):
Linter requires this now. If no use of class stuff, then you have to change to SFC.


Comments from Reviewable

* Generator adds line to spec/rails_helper.rb so that running specs will
  ensure assets are compiled.
* Added more diagnostics for server rendering.
* Calls to setTimeout and setInterval are not logged for server
  rendering unless env TRACE_REACT_ON_RAILS is set to YES.
* Other small changes to the generator including adding necessary npm
  scripts to allow React on Rails to build assets.
* npm modules updated for generator.
* Updated all npm dependencies to latest.
* Update to node 5.10.0 for CI
* Add babel-runtime as a peer dependency of npm module react-on-rails
* Updated generators to have babel-runtime added
@justin808 justin808 force-pushed the add-rails-helper-to-generator branch from b7b2ca4 to fa16c9a Compare April 3, 2016 02:51
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling fa16c9a on add-rails-helper-to-generator into * on master*.

@justin808 justin808 merged commit cf29f7e into master Apr 3, 2016
@justin808 justin808 deleted the add-rails-helper-to-generator branch April 3, 2016 05:55
@jbhatab
Copy link
Member

jbhatab commented Apr 4, 2016

lib/generators/react_on_rails/base_generator.rb, line 194 [r2] (raw file):
I just meant make these checks into two separate checks. So have the rails helper check separate from the spec helper check. It's fine as is thoughl


Comments from Reviewable

@jbhatab
Copy link
Member

jbhatab commented Apr 4, 2016

Reviewed 16 of 19 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

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