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

Ngrx #133

Merged
merged 11 commits into from
Aug 24, 2016
Merged

Ngrx #133

merged 11 commits into from
Aug 24, 2016

Conversation

tja4472
Copy link
Contributor

@tja4472 tja4472 commented Aug 21, 2016

Have added an additional page, components and services for ngrx use.

Based on https://github.com/ngrx/example-app and https://github.com/JavascriptMick/ng2-state-demo.

See ngrx.md for additional references.

@lathonez lathonez mentioned this pull request Aug 21, 2016
@lathonez
Copy link
Owner

Thank you for this PR, which is definitely the most comprehensive I've received on this project.

Frankly I'm a little annoyed as I was looking forward to playing with ngrx myself :)

I want to spend some time to understand the change and how it fits in with Clicker. My goal for #128 was to implement the existing pages with ngrx, rather than implementing new ones. So I'll probably end up removing the existing pages when I merge in (which is a good exercise for me in understanding the change and ngrx, please don't do this yourself!)

If you could fix the linting errors and re-commit so the build passes, that'd be great.

Stephen

@tja4472
Copy link
Contributor Author

tja4472 commented Aug 21, 2016

@lathonez
Copy link
Owner

Sorry only just getting to this.

@lathonez lathonez merged commit 0823756 into lathonez:master Aug 24, 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

2 participants