8000 Add `hasTruthyHashValue` by azdavis · Pull Request #1054 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add hasTruthyHashValue #1054

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
Merged
merged 3 commits into from
Jun 25, 2019
Merged

Add hasTruthyHashValue #1054

merged 3 commits into from
Jun 25, 2019

Conversation

azdavis
Copy link
Contributor
@azdavis azdavis commented Jun 25, 2019

Motivation

I got bitten by this while working on #986.

Before, hasHashValue would return false if the value was present in the hash, but falsy. This is not good when e.g. we have something of the form prop :foo, T::Boolean, default: false and want to know if the key :default is in the rules hash for prop (as I did).

Test plan

A test that essentially tests for this will be added to #986.

@azdavis azdavis requested review from jez and a team June 25, 2019 22:37
@ghost ghost removed their request for review June 25, 2019 22:37
jez
jez approved these changes Jun 25, 2019
View reviewed changes
Copy link
Collaborator
@jez jez left a comment

Choose a reason for hiding this comment

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

oof.

Does this help fix the number of errors we had in the initialize method? Were there a bunch of people using false or nil using default that we weren't creating an OptionalArg for?

@azdavis azdavis force-pushed the azdavis/add-has-truthy-hash-value branch from d99d064 to 01ae292 Compare June 25, 2019 22:43
@azdavis
Copy link
Contributor Author
azdavis commented Jun 25, 2019

It definitely will fix some spurious errors. Waiting on the number for how many.

@azdavis
Copy link
Contributor Author
azdavis commented Jun 25, 2019

It might have fixed around 20.

@azdavis azdavis merged commit 90c20f7 into master Jun 25, 2019
@azdavis azdavis deleted the azdavis/add-has-truthy-hash-value branch June 25, 2019 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jez jez jez 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.
2 participants
@azdavis @jez
0