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 #584 #586

Merged
merged 1 commit into from
Aug 29, 2014
Merged

Fix #584 #586

merged 1 commit into from
Aug 29, 2014

Conversation

paf31
Copy link
Contributor

@paf31 paf31 commented Aug 29, 2014

@garyb Mind reviewing this one?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 948b07d on 584 into 94e0c34 on master.

@garyb
Copy link
Member

garyb commented Aug 29, 2014

I think I understand why that works (and if so, it looks good to me and makes sense), but mind explaining a bit so I'm sure?

@paf31
Copy link
Contributor Author

paf31 commented Aug 29, 2014

Sure. The $? operator applies the current substitution, and starIfUnknown is something we do with kinds to default them to * if there are any unknowns left, since we don't support polymorphic kinds. If starIfUnknown is used before $? then there could be unknowns left which are only unknown because the substitution hasn't been applied yet, which means they aren't really *.

@garyb
Copy link
Member

garyb commented Aug 29, 2014

Great, that's what I thought was happening. I'm just surprised we've not had more problems with kinds already then, I guess * being by far the most common kind means starIfUnknown is usually right though, hiding the problem.

:shipit:

paf31 added a commit that referenced this pull request Aug 29, 2014
@paf31 paf31 merged commit 0be14ac into master Aug 29, 2014
@paf31 paf31 deleted the 584 branch August 29, 2014 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants