Display python debug config ui when editing launch.json#5016
Conversation
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Overall LGTM. I have a few comments below that I'd like you take a look at.
Also, the concept behind IExtensionActivationService is a bit unclear. Are the two new implementations treated sort of like mini-sub-extensions? Doc comments on those classes would really help.
e61a46c to
e17130c
Compare
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Thanks for the fixes. A number of my review comments were marked as resolved but I don't see any corresponding changes and there was no explanation. I've marked those as unresolved. Please at least leave an explanation on why nothing needs to change. Thanks!
2546d70 to
3604dee
Compare
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM, but please make sure we get on the same page before merging.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
Thanks for doing this. I have a few comments on things that are unclear and might need fixing. Also, all those test titles, where the test is targeting an internal method, are still misleading.
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM. That is what I was thinking of, though I'd probably put LaunchJsonUpdaterService above LaunchJsonUpdaterServiceHelper rather than below it. :)
Regardless, thanks for doing this Don. I definitely want to be sure we get on the same page on this stuff though.
ae7127a to
a4e788d
Compare
For #3321
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)