@dpoeschl Anything else you would like?

@dpoeschl can you take a look? i addressed all your feedback i think. Thanks!

So I think we can put this in the feature branch now. And everything in there has been reviewed so getting it into master should be easyish.

However, I'm sending a PR soon to the feature branch as well to fix some licensing things. Let's wait for that before trying to get it into master. 😄

Edit: Unless @jinujoseph dissents. :)

However, I'm sending a PR soon to the feature branch as well to fix some licensing things. Let's wait for that before trying to get it into master. 😄

Sounds good.

Looking at integration test.

Test failure is at: https://ci.dot.net/job/dotnet_roslyn/job/features_embeddedRegex/job/windows_debug_vs-integration_prtest/256/

Retesting to see if its flakeyness or actually product.

@dpoeschl how do i download the test results? i can't figure out jenkins.

test windows_debug_vs-integration_prtest

@CyrusNajmabadi #29001 updated to include this possibly related case, for which the log can be found at https://ci.dot.net/job/dotnet_roslyn/job/features_embeddedRegex/job/windows_debug_vs-integration_prtest/256/artifact/Binaries/Debug/Logs/Microsoft.VisualStudio.LanguageServices.IntegrationTests.dll.out.log, which indicates a VS crash happening at some point...

But I don't see a dump anywhere. @sharwell & @jasonmalinowski were working on the procdump usage in integration tests a while back, though, and I don't know that status.

https://ci.dot.net/job/dotnet_roslyn/job/features_embeddedRegex/job/windows_debug_vs-integration_prtest/ does not seem to be running. Not sure what to do.

test windows_debug_vs-integration_prtest

I think the previous one finally started running about an hour ago. Maybe it didn't retrigger the GitHub UI? I think this is it: https://ci.dot.net/job/dotnet_roslyn/job/features_embeddedRegex/job/windows_debug_vs-integration_prtest/257/ -- it's the right queue and points to the right SHA. If it succeeds I vote we merge.

It passed!

Console output:

15:16:03 Running 2 test assemblies in 2 partitions
15:16:03    2 running,  0 queued,  0 completed
15:17:25    1 running,  0 queued,  1 completed
16:00:40    0 running,  0 queued,  2 completed
16:00:40 ================
16:00:40 Microsoft.CodeAnalysis.Workspaces.MSBuild.UnitTests.dll                     PASSED 00:01:21.5315387  
16:00:40 Microsoft.VisualStudio.LanguageServices.IntegrationTests.dll                PASSED 00:44:36.7094431  
16:00:40 ================
16:00:40 Extra run diagnostics for logging, did not impact run results
16:00:40 Test execution time: 00:44:37.2174516
16:00:40 All tests passed

it's the right queue and points to the right SHA. If it succeeds I vote we merge.

@jinujoseph Any concerns on this goign into the feature branch, and then having the feature branch merge into master?

Yes. The licensing PR coming later today has to happen still.

But into feature branch is 👍 from me.

Great! can you merge in? It will help me in terms of keeping other branches up to date. Thanks much! :)

Merging this into the feature branch without waiting for @jinujoseph because merging into the feature branch is approved and relatively harmless. Of course, any feedback about merging into master (other than the licensing thing) is appreciated still. 😄

Yay! Thanks