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

housekeeping: Add ReactiveCommand usage examples #1919

Merged
merged 3 commits into from Jan 18, 2019

Conversation

worldbeater
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This PR adds usage examples to ReactiveCommand API docs. Addresses reactiveui/rfcs#5 (comment)

What is the current behavior? (You can also link to an open issue here)

There are no examples in our API docs showing up how to create and execute a command.

What is the new behavior?

Examples are present.

@worldbeater worldbeater requested review from a team January 18, 2019 09:47
@codecov
Copy link

codecov bot commented Jan 18, 2019

Codecov Report

Merging #1919 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1919   +/-   ##
=======================================
  Coverage   66.67%   66.67%           
=======================================
  Files         110      110           
  Lines        4372     4372           
  Branches      578      578           
=======================================
  Hits         2915     2915           
  Misses       1287     1287           
  Partials      170      170
Impacted Files Coverage Δ
src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs 73.79% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5b90e7...ccfbd3f. Read the comment docs.

@giusepe
Copy link
Member

giusepe commented Jan 18, 2019

Hey @worldbeater!

Thank you for one more PR :)

One question, instead of using Execute() that doesn't respect the canExecute, shouldn't we use an Observable.Return(42).InvokeCommand(command) in the samples as a way to reinforce it as a best practice?

Copy link
Member

@giusepe giusepe left a comment

Choose a reason for hiding this comment

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

Nice, looks pretty good to me :)

@glennawatson glennawatson merged commit 6f45eec into master Jan 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the rxcmd-usage-examples branch January 18, 2019 15:34
glennawatson pushed a commit that referenced this pull request Mar 23, 2019
* housekeeping: Add ReactiveCommand usage examples

* Prefer InvokeCommand() over Execute()

But mention both aproaches.

* Remove trailing whitespace
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants