Skip to content

Added appEpics#1644

Merged
offtherailz merged 3 commits into
geosolutions-it:masterfrom
kappu72:appEpics
Mar 27, 2017
Merged

Added appEpics#1644
offtherailz merged 3 commits into
geosolutions-it:masterfrom
kappu72:appEpics

Conversation

@kappu72

@kappu72 kappu72 commented Mar 27, 2017

Copy link
Copy Markdown
Contributor

This pull lets pass an object with app specific Epics
If Epic's name, in appEpics, is the same the one in plugins this will override plugin's one

… If Epic's name is the same the one in plugins this will override plugin's one
mbarto
mbarto previously requested changes Mar 27, 2017
const ListenerEnhancer = require('@carnesen/redux-add-action-listener-enhancer').default;

module.exports = (initialState = {defaultState: {}, mobile: {}}, appReducers = {}, plugins, storeOpts) => {
module.exports = (initialState = {defaultState: {}, mobile: {}}, appReducers = {}, appEpics = {}, plugins, storeOpts) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

New arguments at the end, for compatibility reasons, please

@offtherailz offtherailz Mar 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This signature is better. This doesn't break the binding with StandardApp or other apps. I should put it in that place instead of the bottom.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Binding appEpics wouldn't be possible

@offtherailz offtherailz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove

});
it('combineEpics', () => {
const plugins = {MapSearchPlugin: MapSearchPlugin};
const appEpics = {appEpics: (actions$) => actions$};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This sounds like an infinite loop. Is not executed but it should. Please remove it.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.5%) to 77.08% when pulling ad35ca8 on kappu72:appEpics into ffe8487 on geosolutions-it:master.

@offtherailz offtherailz dismissed mbarto’s stale review March 27, 2017 13:45

as discussed we removed this because is the correct usage

@offtherailz offtherailz merged commit 8617224 into geosolutions-it:master Mar 27, 2017
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.5%) to 77.08% when pulling 3e95ba1 on kappu72:appEpics into ffe8487 on geosolutions-it:master.

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.

4 participants