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

Allowing ability to observe partial state. #7

Merged
merged 4 commits into from
Mar 20, 2018

Conversation

mhusainisurge
Copy link
Contributor

This is to implement enhancement #4. I generalized it a bit to allow using any arbitrary selector on the state. This is the my first contribution on Github, so I hope I got everything right. 😃

public IObservable<object> ObserveAction()
{
return _actionSubject.AsObservable();
}

Copy link
Owner

Choose a reason for hiding this comment

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

Overloaded methods (with the same name) should be close to one another. The same should be applied for ObserveState.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@Odonno
Copy link
Owner

Odonno commented Mar 19, 2018

@mhusainisurge Ey. Thanks for your first time contribution! I really enjoy seeing someone with interest on this project.

Your code looks good. Will need to update the docs in consequence, it is no a subject on this PR so I don't force you with it. Anyway, if you make the changes on the line return on the file, I'll consider this PR OK and merged it right after.

On the meantime, will figure out why appveyor builds are failing..

@Odonno
Copy link
Owner

Odonno commented Mar 19, 2018

Ok, fixed. Build is now working like expected.

@mhusainisurge
Copy link
Contributor Author

Thanks for considering my PR. I have implemented the changes requested. Looking forward to more contributions!

@Odonno Odonno merged commit 280ae2a into Odonno:master Mar 20, 2018
@mhusainisurge mhusainisurge deleted the feature/partial-state branch March 25, 2018 17:24
@Odonno Odonno added the feature New feature or request label Mar 25, 2018
@Odonno Odonno added this to the 1.1 milestone Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants