Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Dec 7, 2016

This fixes a bunch of issues with error reporting in scripts

The second was caused by recent F# 4.1 fixes to replay errors from the load closure computation. We need to be more selective about which errors we replay (do not replay parsing errors, which may not be valid for cached entries in the loadClosure cache).

We also had a bad cache key in script LoadClosure computation (we were not keying on the full set of #load directives which support the LoadClosure computation, and were not keying on unresolved assembly references)

Some testing needs to be added, will look at that tomorrow

I added some cleanup for diagnostics processing in general, unifying the "Error" and "Warning" paths into single "Diagnostic" paths, passing the isError parameter. Some paths were using warn == not isError and these are switched around to just use isError

@KevinRansom
Copy link
Contributor

@dsyme Please don't Pull any PR's for a while.

@forki
Copy link
Contributor

forki commented Dec 7, 2016

@KevinRansom couldn't just use another branch for release preparation? This would not stop the time for others.

@KevinRansom
Copy link
Contributor

@forki we could have, and will next time around ... it's just that the insanity of the last few weeks meant we wanted to stay with what we had until too late to change.

@forki
Copy link
Contributor

forki commented Dec 7, 2016 via email

@dsyme
Copy link
Contributor Author

dsyme commented Dec 7, 2016

@KevinRansom No problem.

  • If you could use an rc2 branch (git push visualfsharp master:rc2) that would be great but I'm totally ok either way.
  • I pushed a branch bleeding-edge containing a bunch of new the features and fixes. I'll aim to keep it integrated and use it a bit. It won't necessarily pass tests and shouldn't be used as a development head, just to build a set of tools to trial new features together

@KevinRansom
Copy link
Contributor

We will use rc3 for the next release and master. Brett has to generate the necessary VSO build goop ... and we will be golden.

@KevinRansom
Copy link
Contributor

@dotnet-bot test this please

1 similar comment
@KevinRansom
Copy link
Contributor

@dotnet-bot test this please

@KevinRansom KevinRansom mentioned this pull request Dec 22, 2016
@dsyme
Copy link
Contributor Author

dsyme commented Dec 27, 2016

@KevinRansom I'm bringing this PR up-to-date, thanks

@KevinRansom
Copy link
Contributor

@dotnet-bot test this please

@KevinRansom KevinRansom merged commit ee84eb7 into dotnet:master Dec 29, 2016
@cartermp
Copy link
Contributor

Woot! Happy to see this merged.

@dsyme
Copy link
Contributor Author

dsyme commented Dec 29, 2016

Super, thanks for merging

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.

5 participants