Skip to content

chore(package): update package-lock#3277

Merged
kwonoj merged 4 commits into
ReactiveX:masterfrom
kwonoj:chore-pkg-lock
Feb 12, 2018
Merged

chore(package): update package-lock#3277
kwonoj merged 4 commits into
ReactiveX:masterfrom
kwonoj:chore-pkg-lock

Conversation

@kwonoj

@kwonoj kwonoj commented Jan 31, 2018

Copy link
Copy Markdown
Member

Description:
This PR pins all of our dependncies, also check in package-lock.json. Now package lock support becomes quite stable, it works on cross-platform without modifying dependency when just try to install dependencies. We may need bump up huge number of dependencies in here, but that's different story.

Related issue (if exists):

@kwonoj kwonoj requested a review from benlesh January 31, 2018 00:43
@rxjs-bot

rxjs-bot commented Jan 31, 2018

Copy link
Copy Markdown
Warnings
⚠️

❗ Big PR (1)

Messages
📖

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

CJS: 1358.3KB, global: 726.5KB (gzipped: 117.3KB), min: 140.3KB (gzipped: 30.6KB)

Generated by 🚫 dangerJS

@kwonoj

kwonoj commented Jan 31, 2018

Copy link
Copy Markdown
Member Author

🤔 just pinning dependency makes test fails?

@cartant

cartant commented Jan 31, 2018

Copy link
Copy Markdown
Collaborator

Were they pinned to the most-recent, compatible versions? Or to potentially old versions?

@kwonoj

kwonoj commented Jan 31, 2018

Copy link
Copy Markdown
Member Author

pin just removed caret from version range. There should be some semver minor bumps with carets, but still making test fails seems bit odd.

@cartant

cartant commented Jan 31, 2018

Copy link
Copy Markdown
Collaborator

How did that test ever pass? The implementation seems to explicitly not use the scheduler if there is only one observable?

if (observables.length === 1 || (observables.length === 2 && isScheduler(observables[1]))) {
  return from(<any>observables[0]);
}

This is the logged output:

  2517 passing (4s)
  4 pending
  1 failing
  1) Observable.concat should use the scheduler even when one Observable is concat'd:
     AssertionError: expected true to be false
      at Context.<anonymous> (spec/observables/concat-spec.ts:369:31)

And this is the failing test.

@kwonoj

kwonoj commented Jan 31, 2018

Copy link
Copy Markdown
Member Author

/cc @benlesh , I recall this test added recently.

@kwonoj

kwonoj commented Feb 12, 2018

Copy link
Copy Markdown
Member Author

The implementation seems to explicitly not use the scheduler if there is only one observable?

that's true, test case description is reading bit confusing. It concat's one observable + scheduler as param, which takes execution path for arguments length as > 1 .

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 97.502% when pulling 35058a3 on kwonoj:chore-pkg-lock into 94156a2 on ReactiveX:master.

@kwonoj

kwonoj commented Feb 12, 2018

Copy link
Copy Markdown
Member Author

phew, bit larger work than I thought.

@kwonoj kwonoj merged commit 92dd3d1 into ReactiveX:master Feb 12, 2018
@kwonoj kwonoj deleted the chore-pkg-lock branch February 12, 2018 18:38
@lock

lock Bot commented Jun 6, 2018

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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.

Update dependency-cruiser

4 participants