Skip to content

Fixes #583.#613

Closed
isaacabraham wants to merge 1 commit into
dotnet:masterfrom
isaacabraham:master
Closed

Fixes #583.#613
isaacabraham wants to merge 1 commit into
dotnet:masterfrom
isaacabraham:master

Conversation

@isaacabraham

Copy link
Copy Markdown
Contributor

No description provided.

@msftclas

msftclas commented Sep 4, 2015

Copy link
Copy Markdown

Hi @isaacabraham, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@enricosada

Copy link
Copy Markdown
Contributor

Build execution time has reached the maximum allowed time for your plan (40 minutes).

@KevinRansom can you try to rerun it? or @isaacabraham push something to trigger another time, usually the build is < 40min

@latkin

latkin commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

Looks good, just needs a test case.

@dsyme included a minimal repro in #583, I would suggest adding it as a new "pos" case at tests\fsharp\typecheck\sigs

@isaacabraham

Copy link
Copy Markdown
Contributor Author

ok, will do

@isaacabraham

Copy link
Copy Markdown
Contributor Author

Something isn't right here - I'm running the fsharp tests with RunTests.cmd debug fsharp and getting 4 tests passed, 100 tests failed. Is there something else I need to run beforehand? The actual main solution all builds in VS.

@latkin

latkin commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

@isaacabraham take a look at everything that gets built in appveyor-build.cmd. Build on the command line, not in VS.

And unless I'm actually planning to attach a debugger, I typically build/test in release mode, it cuts down test execution time significantly. Up to you.

@isaacabraham

Copy link
Copy Markdown
Contributor Author

Ok - thanks for the advice re:release mode. I'm on Win10 at the moment so the appveyor build doesn't work. I'll try again on Sunday on my other machine.

@latkin latkin closed this in 95ba9c8 Sep 16, 2015
@latkin latkin added the fixed label Sep 16, 2015
@latkin latkin added this to the F# 4.0 Update 1 milestone Sep 16, 2015
@latkin

latkin commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

I went ahead and added the test case when I merged. Thanks!

@isaacabraham

Copy link
Copy Markdown
Contributor Author

Thanks for doing that - I'm out of action currently and can't code for the next few days, cheers.

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.

4 participants