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

Actioncable - React #355

Merged
merged 20 commits into from
Jan 11, 2017
Merged

Actioncable - React #355

merged 20 commits into from
Jan 11, 2017

Conversation

richa
Copy link
Contributor

@richa richa commented Jan 6, 2017

This change is Reviewable

@justin808
Copy link
Member

@richa We need CI to pass.

Linter errors

48 files inspected, no offenses detected
Running ruby-lint Linters via ruby-lint app config spec lib
ruby-lint app config spec lib
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.2-compliant syntax, but you are running 2.3.1.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Running eslint via cd client && $(npm bin)/eslint . -- --ext .jsx,.js
cd client && $(npm bin)/eslint . -- --ext .jsx,.js
/home/travis/build/shakacode/react-webpack-rails-tutorial/client/app/bundles/comments/reducers/commentsReducer.js
40:27 error Unexpected block statement surrounding arrow body arrow-body-style
40:102 error Expected '===' and instead saw '==' eqeqeq
40:169 error Missing semicolon semi
✖ 3 problems (3 errors, 0 warnings)
rake aborted!
Command failed with status (1): [cd client && $(npm bin)/eslint . -- --ext ...]
/home/travis/build/shakacode/react-webpack-rails-tutorial/lib/tasks/linters.rake:43:in block (2 levels) in <top (required)>' /home/travis/.rvm/gems/ruby-2.3.1/gems/rake-11.3.0/exe/rake:27:in <top (required)>'
/home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in eval' /home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in

'
Tasks: TOP => default => ci => ci:all => lint => lint:lint => lint:js => lint:eslint
(See full trace by running task with --trace)

@justin808
Copy link
Member

Left some comments.


Reviewed 21 of 22 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Gemfile, line 110 at r2 (raw file):

group :production do
  gem 'rails_12factor'

why did you add this?

I'm pretty sure this is not needed for rails 5.


client/app/bundles/comments/components/CommentBox/CommentBox.jsx, line 45 at r2 (raw file):

    fetchComments();
    this.subscribeChannel();
    //this.intervalId = setInterval(fetchComments, this.props.pollInterval);

remove comment


config/secrets.yml, line 19 at r2 (raw file):

test:
  secret_key_base: 1ab8adbcf8410aebbce9b6dd6db7b5d090297bd22cf789b91ff44ae02711e8c128453d3e5c97eadf9066efe1a1e0dc1921faf7314d566c114d3ed60ae7ea614c
  action_cable_url : <%= ENV["SERVER_PORT"] %>

why should development and test need this to be anything other than localhost?


Comments from Reviewable

@justin808
Copy link
Member

:lgtm:


Reviewed 4 of 4 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@justin808 justin808 merged commit 6be99d6 into shakacode:master Jan 11, 2017
@justin808
Copy link
Member

Thanks @richa. Nice job!

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.

None yet

2 participants