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

Add react-router-redux #93

Closed
wants to merge 11 commits into from
Closed

Add react-router-redux #93

wants to merge 11 commits into from

Conversation

carloscuatin
Copy link
Contributor

add react-router-redux and refactor code and test for new features as jwt auth

* upstream/master:
  #77 Remove extra call from fetchData
  Small tweak to add post reducer
  Update index.js with devToolsExtenstion
* upstream/master:
  closes #51 closes #52 closes #85
  Update README.md
* upstream/master:
  devtools fix
  fixing minor spelling mistake in test component spec from creame to create
  Update README.md
@SOSANA
Copy link
Contributor

SOSANA commented Mar 3, 2016

hey @carloscuatin

was checking this out and looks good.

two things I noticed...

In /PostContainer/PostContainer.jsx line 46 put closing tag on same line as nothing changed

I believe if nodemon isn't installed globally we may have problems with 'rs' server after changes, I believe we need a nodemon.json file but can't be sure as I have it personally installed globally I just seen in other cases people having problems without having nodemon.json file settings when installed locally. check this out here for nodemon FAQ.

in shared/redux/reducers/blog.js maybe we should rename this to post.js reason is maybe in future we add comments.js, user.js, etc blog.js sounds to broad of a term that could cover all these as a whole.

Also we should change shared/redux/actions/actions.js to shared/redux/actions/post.js ? same concept I mentioned above for reducers we could apply to actions. Maybe this one we do on another PR cause we need to change components with no sub folders just name of file :)

What sort of implementation did for refactor code and test for new features as jwt auth? are you considering that jwt auth redux library? or passport?

@prank7 @mayankchd @sandeeppanda92 @carloscuatin thoughts?

Nice work brother

@SOSANA SOSANA mentioned this pull request Mar 3, 2016
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

3 participants