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.

Sign up for GitHub

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

Jump to bottom

feat(scheduled): Adds scheduled creation method #4595

Merged
merged 1 commit into from
Apr 23, 2019
Merged

feat(scheduled): Adds scheduled creation method #4595

merged 1 commit into from
Apr 23, 2019

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Feb 26, 2019 •
edited
Loading

The scheduled creation method is meant to create an observable with a scheduler. The goal of this PR is to set us up to remove scheduling from very common code paths like from and of, amongst others.

This deprecates versions of from, concat, merge, startWith and endWith that allow passing a scheduler.

This PR also simplifies a few common code paths.

Put another way, this PR:

  • Splits off the scheduled code paths for various branches of from into their own functions
  • Updates a bunch of scheduled versions of functions as deprecated
  • Cleans up some scheduling-related code

@benlesh benlesh changed the title feat(scheduled): Adds schduled creation method feat(scheduled): Adds scheduled creation method Feb 26, 2019
@rxjs-bot
Copy link

Warnings
⚠️

❗ Big PR (1)

(1) : Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.

Generated by 🚫 dangerJS

@coveralls
Copy link

coveralls commented Feb 26, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 8384

  • 35 of 37 (94.59%) changed or added relevant lines in 13 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 96.6%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/internal/observable/fromObservable.ts 0 1 0.0%
src/internal/util/subscribeToResult.ts 3 4 75.0%
Files with Coverage Reduction New Missed Lines %
src/internal/observable/fromObservable.ts 2 0.0%
Totals Coverage Status
Change from base Build 8381: -0.07%
Covered Lines: 5200
Relevant Lines: 5383
💛 - Coveralls

cartant
cartant approved these changes Apr 7, 2019
View reviewed changes
Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

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

LGTM

@ci-reporter
Copy link

ci-reporter bot commented Apr 23, 2019

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Travis CI build is failing as of e47abac. Here's the output:

npm test
> @reactivex/[email protected] test /home/travis/build/ReactiveX/rxjs
> cross-env TS_NODE_PROJECT=spec/tsconfig.json mocha --opts spec/support/default.opts "spec/**/*-spec.ts"



  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․,
  ,,․․․,․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․���․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․���․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․!․․․․․․․․․․․․․․․․․

  2572 passing (3s)
  4 pending
  1 failing

  1) subscribeToResult
       should synchronously complete when subscribe to scalarObservable:
     AssertionError: expected { Object (closed, _parentOrParents, ...) } to not exist
      at spec/util/subscribeToResult-spec.ts:17:32
      at Context.modified (spec/helpers/testScheduler-ui.ts:203:13)

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@benlesh
feat(scheduled): Add scheduled creation method
5ed8302 
- Simplifies code around common observable creation and subscription
- Removes `scalar` internal impl
- Deprecates a number of APIs that accept schedulers where we would rather people use `scheduled`.
@benlesh benlesh merged commit f57e1fc into ReactiveX:master Apr 23, 2019
@dmytro-gokun dmytro-gokun mentioned this pull request Apr 23, 2019
Closed
@mtraynham mtraynham mentioned this pull request Apr 24, 2019
Merged
@DaSchTour
Copy link

Is there some documentation how migration from of, from to scheduled looks like?

BioPhoton pushed a commit to BioPhoton/rxjs that referenced this pull request May 15, 2019
@benlesh @BioPhoton
feat(scheduled): Add scheduled creation method (ReactiveX#4595)
b4733d7 
- Simplifies code around common observable creation and subscription
- Removes `scalar` internal impl
- Deprecates a number of APIs that accept schedulers where we would rather people use `scheduled`.
@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@martinsik martinsik martinsik left review comments

@cartant cartant cartant approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@benlesh @rxjs-bot @coveralls @DaSchTour @martinsik @cartant