-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
remove workbench/contrib/experiments #185658
Conversation
@isidorn please consider to take a moment to drill a bit into, we have all the diag available. here in this case I see a lot of native crashes: You can report these to Deepak. |
@bpasero can you clarify what I should do here? Where do you see native crashes? And should I create new issues for Deepak? |
@isidorn you need to drill into the attachments of the build to find all the logs and crash dumps. This shows logs for all tests: If crash dumps have any size > 100kb, thats a good hint that there are native crash dumps: You can then file an issue with the crash dump attached. There are many here so maybe picking one is sufficient. Btw this does not mean these crashes are related to your change. |
Thanks Ben! I just filled #186175 |
@bpasero sorry I got stuck. I ran the remote smoke tests locally and I can indeed reproduce the problem. I ran them via When I run it locally I see that element exists. Do you have any hints what might be going on? |
@isidorn please check for each file you delete if any code still depends on any of it. For example if you search for |
@bpasero thanks. I did that already, but I double checked now and no files depend on file names I deleted. I also did a full text search and we are good - nothing found. As for the remaining instances of enableExperiments, that is for the AssignementService. As far as I understand we still need that one for our new way of running experiments. @sbatten is the owner of that one and he might confirm. |
i brought it up because I think the smoke tests then ran for me when removing that. I have a feeling that the remote server fails to start in the smoke tests, you can see how the window quickly closes while the remote is setting up. I was not able to find the underlying reason though, but maybe its related to that setting. |
I think I found the issue. I removed the registration of So thanks Ben and the smoke tests 👏 |
Ok. Tests passed. @bpasero can you just approve via review. Thank you! I am having follow up discussions with Logan to see if we really need the |
@lramos15 confirmed that we indeed need the AssignmentService for our new experimentation framework. |
Ref https://github.com/microsoft/vscode-internalbacklog/issues/2855
Removes unused
workbench/contrib/experiments
All tests are green and I ran VS Code out of Source and it looks OK.
fyi @bpasero