Skip to content

feat(scheduled): Adds scheduled creation method - #4595

Merged
benlesh merged 1 commit into
ReactiveX:masterfrom
benlesh:fromScheduled
Apr 23, 2019
Merged

feat(scheduled): Adds scheduled creation method#4595
benlesh merged 1 commit into
ReactiveX:masterfrom
benlesh:fromScheduled

Conversation

@benlesh

@benlesh benlesh commented Feb 26, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

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

rxjs-bot commented Feb 26, 2019

Copy link
Copy Markdown
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

coveralls commented Feb 26, 2019 •
edited
Loading

Copy link
Copy Markdown

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

martinsik
martinsik reviewed Feb 27, 2019
View reviewed changes
Comment thread src/internal/observable/empty.ts
cartant
cartant approved these changes Apr 7, 2019
View reviewed changes

@cartant cartant left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@ci-reporter

ci-reporter Bot commented Apr 23, 2019

Copy link
Copy Markdown

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/rxjs@6.4.0 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

DaSchTour commented May 7, 2019

Copy link
Copy Markdown

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

@cartant cartant cartant approved these changes
+1 more reviewer
@martinsik martinsik martinsik left review comments
Reviewers whose approvals may not affect merge requirements

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