Skip to content

Conversation

@matthid
Copy link
Contributor

@matthid matthid commented Mar 10, 2018

fixes #3752

Copy link
Contributor

@dsyme dsyme left a comment

Choose a reason for hiding this comment

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

This looks good and I'm glad to see you found the cause of the issue.

Just one main comment - any reason you're not using a negNN.fs test?


[<Test>]
let ``typeinferenceWarning`` () =
let cfg = testConfig "core/typeinferenceWarning"
Copy link
Contributor

Choose a reason for hiding this comment

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

Normally we'd add a negNN.fs test for such a case? Is there any reason not to be doing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reason is that I have no idea what I’m doing but I still wanted to add a test ;)

Is there docs around that or can someone help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is "negNN"?

Copy link
Contributor

Choose a reason for hiding this comment

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

typecheck/sigs/neg101.fs etc. for example

@matthid
Copy link
Contributor Author

matthid commented Mar 12, 2018

@dsyme Thanks, this makes the test-code a bit simpler :)
Lets see if it still works.

@matthid
Copy link
Contributor Author

matthid commented Mar 14, 2018

@dsyme Ok I think this is ready.

@KevinRansom
Copy link
Contributor

@matthid Thanks for this

@KevinRansom KevinRansom merged commit 178fc31 into dotnet:master Mar 17, 2018
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.

Some warnings are missing

3 participants