Skip to content

Conversation

@SethDavenport
Copy link
Member

Fixes #314

@SethDavenport SethDavenport force-pushed the docs/intro-tutorial.md branch from ab39dbb to bc6ff17 Compare February 5, 2017 20:19
@SethDavenport
Copy link
Member Author

SethDavenport commented Feb 5, 2017

@rajinder-yadav this was something you mentioned a while back. Thoughts?

@SethDavenport
Copy link
Member Author

SethDavenport commented Feb 6, 2017

Note to self:

  • fix some typos
  • redux-beacon is another cool middleware example
  • is it worth talking about combineReducers?
  • if we put dispatch in the action service then the component is much cleaner
  • Declare the IAppState vars as readonly?

README.md Outdated

Redux is a popular approach to managing state in applications. It emphasises:

* A single, immutable data store
Copy link
Contributor

@rajinder-yadav rajinder-yadav Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a proper sentence, it's a good idea to add a period. You can omit it, but stay consistent since the last bullet has a period.

README.md Outdated
into your Angular 2+ applications. Our approach helps you by bridging the gap
with some of Angular's advanced features, including:

* Change processing with RxJS observables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about add or omitting a period.

* Compile time optimizations with `NgModule` and Ahead-of-Time compilation
* Integration with the Angular change detector.

## Getting Started
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What style of heading are you following. Two popular one are:

  1. Capitalize each word
  2. Capitalize first word and only "nouns". (See to be preferred for published articles).

I would go over the sub heading to make sure they are consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I will stick with English Title Case rules. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although strictly speaking title case is typically defined in English as capitalizing everything except prepositions, articles, and pronouns :)

import reduxLogger from 'redux-logger';
import { rootReducer } from './reducers';

interface IAppState { /* ... */ };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to expand on how the interface IAppState comes into play. For single state or multiple states. Later how those states are accessed, if they are separate groups of states.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll add a note. However I didn't want to get into too complicated an example in this tutorial. I might write a second chapter later.

})
export class AppModule {
constructor(ngRedux: NgRedux<IAppState>) {
ngRedux.configureStore(rootReducer, {}, [ createLogger() ]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found configureStore initially confusing, the API and what each argument signifies. Would be nice to expand and explain it here quickly.

@rajinder-yadav
Copy link
Contributor

Looks goods, just a few comments.

@SethDavenport SethDavenport merged commit e89910d into angular-redux:master Feb 7, 2017
@SethDavenport SethDavenport deleted the docs/intro-tutorial.md branch February 7, 2017 03:25
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.

2 participants