Skip to content
This repository was archived by the owner on Oct 1, 2018. It is now read-only.

docs(operators): add documentation for takeWhile #233

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

ashwin-sureshkumar
Copy link
Collaborator

Close #103

@codecov-io
Copy link

codecov-io commented Jan 12, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #233   +/-   ##
=======================================
  Coverage   77.14%   77.14%           
=======================================
  Files          15       15           
  Lines         175      175           
  Branches        7        7           
=======================================
  Hits          135      135           
  Misses         40       40

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 65c2aef...384015f. Read the comment docs.

Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

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

Some small "changes". Rest LGTM

name: 'takeWhile',
operatorType: 'filtering',
signature:
'public takeWhile(predicate: function(value: T, index: number): boolean): Observable<T>',
Copy link
Member

Choose a reason for hiding this comment

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

@btroncone suggested to remove the generic from the signature

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jwo719 @btroncone - Generic on the return type ? Just so I am on the page.

Copy link
Member

Choose a reason for hiding this comment

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

if i understand @btroncone at #228 correctly, yes

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ashwin-sureshkumar @jwo719 Yes, let's leave out in signatures per #196 (comment)

{
type: 'Tip',
text: `
Takes values from the source only while they pass the condition given. When the first value does not satisfy, it completes.
Copy link
Member

Choose a reason for hiding this comment

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

I think the tip is pretty redundant to the description above.

takeWhile(ev => ev.clientX > 100)
);
result.subscribe(x => console.log(x));
`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here sample output?

@ashwin-sureshkumar
Copy link
Collaborator Author

@jwo719 @sumitarora - Done, please re-review.

Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

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

LGTM

@sumitarora sumitarora merged commit 6a90120 into ReactiveX:master Feb 5, 2018
@ladyleet
Copy link
Member

ladyleet commented Feb 5, 2018

yay thanks @sumitarora for merging!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants