[Don Jayamanne (DonJayamanne)]

For #3321

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Has a news entry file (remember to thank yourself!)
• Has sufficient logging.
• Has telemetry for enhancements.
• Unit tests & system/integration tests are added/updated
• [n/a] Test plan is updated as appropriate
• [n/a] package-lock.json has been regenerated by running npm install (if dependencies have changed)
• [n/a] The wiki is updated with any design decisions/details.

[Eric Snow (ericsnowcurrently)]

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.

[Eric Snow (ericsnowcurrently)]

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!

[Eric Snow (ericsnowcurrently)]

LGTM, but please make sure we get on the same page before merging.

[Eric Snow (ericsnowcurrently)]

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.

[Eric Snow (ericsnowcurrently)]

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.