-
Notifications
You must be signed in to change notification settings - Fork 62
docs(operators): add fromPromise documentation #229
base: master
Are you sure you want to change the base?
docs(operators): add fromPromise documentation #229
Conversation
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
=======================================
Coverage 90.49% 90.49%
=======================================
Files 115 115
Lines 442 442
Branches 10 10
=======================================
Hits 400 400
Misses 40 40
Partials 2 2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small typo in description, other than that looks great! 👍
type: 'Scheduler', | ||
attribute: 'optional', | ||
description: | ||
'An optional IScheduler to use for scheduling the delivery of the resolved value (or the rejection).s' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think there's a small typo at end of sentence (or the rejection).s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, you're right. Thanks for the hint. Will fix it this afternoon.
fix typo in the description of fromPromise operator
@btroncone - Could you please re-review ? |
@ashwin-sureshkumar any idea why travis is broken? there is no log or something like that |
thx @jwo719 @ashwin-sureshkumar @btroncone for all your hard work! |
name: 'Convert the Promise returned by Fetch to an Observable', | ||
code: ` | ||
const result = Rx.Observable.fromPromise(fetch('http://myserver.com/')); | ||
result.subscribe(x => console.log(x), e => console.error(e));`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change these to es6 imports, there is an incoming PR from @btroncone converting the existing examples.
@btroncone @ashwin-sureshkumar last commit should fix both requested changes. Please take a look at it and let me know if there is something else I can fix. |
code: ` | ||
import {fromPromise} from 'rxjs/observable/fromPromise'; | ||
|
||
const result = fromPromise(fetch('http://myserver.com/')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use an actual API example here github
or any other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one should be fixed
import {fromPromise} from 'rxjs/observable/fromPromise'; | ||
|
||
const result = fromPromise(fetch('http://myserver.com/')); | ||
result.subscribe(x => console.log(x), e => console.error(e));`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add expected output also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be rather confusing in this case. Because the operator is more or less like a cast and doesn't manipulate the output. What do you think? Of course I can add an output, but I think it won't add much value in this case.
@btroncone, @ashwin-sureshkumar, @sumitarora could you take another look at this pr to get this done? |
closes: #79