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 example app #138

Closed
wants to merge 5 commits into from
Closed

Add example app #138

wants to merge 5 commits into from

Conversation

Norris1z
Copy link
Contributor

This pr adds demo apps which guides users on how to use this package with and without redux.

This pr if merged will solve issues #21

Norris1z and others added 5 commits December 24, 2018 05:17
When using this library with redux saga, the action:
`@@network-connectivity/CONNECTION_CHANGE` seems to be fired even when the `internetConnection` state is the same as the `previousState`.

This pull request fixes rgommezz#119 

This pull request if merged will fire connection change action only when the current action is different from the previous action
@Norris1z
Copy link
Contributor Author

This will also solve #118

@rgommezz
Copy link
Owner

Hey @Norris1z,

I have mixed feelings about this PR.

On one side, I appreciate the effort of creating an example application that showcases how to use the library in practise. It's clearly one of the important things missing in this repository and that's great for the library itself.

But on the other side, a PR like this size and nature has to be consulted beforehand with the library owner. I don't know if you've read the contributing guide, but I recommend you do every time you plan to contribute to some other OSS library from now on, in order to not waste anyone's precious time, most importantly yours.

Now, being that said, one of the problems with this PR is that it's been developed while I was working on the next major release of the library, that has re-structured the whole repository organisation and introduced several breaking changes, with a brand new API, therefore making some of your examples stale.

In addition to that, a work of this nature is probably better fit for the library owner. I feel the examples are too basic and some of the code is already reflected as snippets in the README.md. Besides making your without-redux example obsolete, There is no example for thunks either and the action queue example has to reflect visually how the actions are added/dispatched/dismissed. As you may understand, I am very biased about how an example app should look like for this library.

I am truly sorry, but I am afraid I can't merge this. I really hope this won't happen again in your OSS experience, nor changing your desire of keeping contributing.

@rgommezz rgommezz closed this Dec 28, 2018
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