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.

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

fix(pluck): key union type strictness #4585

Merged
merged 1 commit into from May 16, 2019

Conversation

@imcotton
Copy link
Contributor

commented Feb 23, 2019

Description:
Fixing the overloading type define off-by-one being widen

@imcotton

This comment has been minimized.

Copy link
Contributor Author

commented Feb 23, 2019

Wait... does the loosely typed key inputs actually desired behavior? I mean it's valid JavaScript runtime code but shall the TS add the extra checking over here?

@imcotton imcotton force-pushed the imcotton:fix-pluck branch 2 times, most recently from f47b849 to 69d416b Feb 23, 2019
@coveralls

This comment has been minimized.

Copy link

commented Feb 23, 2019

Pull Request Test Coverage Report for Build 8147

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.962%

Totals Coverage Status
Change from base Build 8136: 0.0%
Covered Lines: 5777
Relevant Lines: 5958

💛 - Coveralls
@benlesh

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

this change is likely to break a few people, so I'm going to target it at the next major version

@imcotton imcotton force-pushed the imcotton:fix-pluck branch from 69d416b to 548e665 Feb 27, 2019
@imcotton

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2019

target it at the next major version

Good call.

BTW, I've made rebase & squash for easy picking up later on.

@benlesh benlesh merged commit bd5ec2d into ReactiveX:master May 16, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 96.962%
Details
@lock lock bot locked as resolved and limited conversation to collaborators Jun 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.