Christoph Bergmeister (@bergmeister) this looks promising :-)

Ryan Yates (@kilasuit) Please see my 2 recent review comments. I thought the change was easy to make but it turned out to be more complex than I initially thought and wasn't always working as expected, hence why it is still in draft, but thanks for the ping as I haven't looked at it for a few months.

Update: I still need to fix the build failure due to the missing component registration (trivial but not on a PC atm) but apart from that I don't plan to push too many more changes.
Will post results and different scenarios of manual testing. What I think should happen at the moment is that if the checkbox is ticked (or MSI switch with value 1 is passed), then the environment variable will be set to 1. On uninstall, the environment variable should be removed. At the moment the GUID is hard coded, which means one would have to uninstall stable and preview to remove the environment variable because MSI keeps a component ref count. Maybe that's a reason why we should let WIX autogenerate a unique component guid for every installation/msi but that also mean we'd have to check/test what the behaviour is during MSI upgrade.

This is the final implementation and it's ready to be merged. I've tested it and scenarios are:

• On uninstall, the environment variable gets removed
• If the checkbox is ticked, then the env variable gets set to 1. If it is not ticked, no action is being performed, i.e. there is no action to remove/change the env variable if it is already pre-defined
• Given one installs one version (e.g. stable) and disables telemetry and then installs preview but does not disable telemetry: If one then uninstalls stable, the telemetry env variable gets removed. This was surprising at first as I thought that it would only get removed once preview got uninstalled as well due to the component GUID being hard-coded but that is probably not the case because the parent directory is different. This is good behaviour and better than I expected.

cc Joey Aiello (@joeyaiello) to review the language

my response to the tool-tip for telemetry is that it is over long and over wrought. The whole reboot pending sentence seems unneeded. Also, it's very engineer centric - we should be redirecting to documentation so those that want more information can get it. I think that we need to short 2 sentences: Processes may need to be restarted for the setting to take effect. See for more information.

My use of may is deliberate. We needn't talk about WM_SETTINGSCHANGE in the tool-tip.
I don't think this is ready as is.

Fair point James Truher (@JamesWTruher) about it being the message being to technical. Because not many people read the tooltip, I thought I'd be very explicit but I went a bit overboard with it but WiX/MSI at least provides line wrapping in case there is not enough horizontal space.
Is the text in parenthesis at least OK?
The problem with WiX is that it cannot provide a tooltip that can be copy-paste able or have a click-able URL because otherwise I'd have linked to this: https://wixtoolset.org/documentation/manual/v3/customactions/wixsettingchange.html
Do we still need the tooltip then? Happy to accept and apply any suggestion you have as-is.

WiX Toolset

WiX toolset

WixBroadcastSettingChange and WixBroadcastEnvironmentChange Custom Actions

Heath Stewart (@heaths) Can you please review as well from a WiX point of view. I see in your recent PR you removed some of the GUIDs but in this case, WiX would complain when removing the GUID or using an autogenerated one...

Never ending story ... how serious is this whole debacle? I started another issue at least 3+ years ago about opt-out being ignored - and this is still the case: I can post Netlimiter prompts from pwsh.exe process to whomever is interested.
I was told back then that it has to be, quote: "some other component", despite the process trying to send "something" being pwsh.exe. How do I opt out of "some component", btw?

It would be cool if someone could explain to me what the problem is and why its taking you guys so long

So how about it NOW?

Christoph Bergmeister (@bergmeister) We need some time to review internally for some compliance issue(s). Sorry, the timeline on these things are unpredictable.

I've pinged the PM team to get the compliance review to happen.

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Compliance has approved the wording, should be good to merge

Note to test this with silent update

Heath Stewart (@heaths) I think it was fine before looking at the tests. Some of the other properties, need the search pattern, but this one seems to persist even without the search, and I think simpler is better.

Travis Plunk (@TravisEz13) didn't you still want to read the telemetry setting into DISABLE_TELEMETRY, or was that a separate property? I see in the original PR it was something like POWERSHELL_TELEMETRY_OPTOUT.

This PR has 29 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Label      : Extra Small
Size       : +28 -1
Percentile : 11.6%

Total files changed: 2

Change summary by file extension:
.wxs : +7 -1
.ps1 : +21 -0

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Heath Stewart (@heaths) DISABLE_TELEMETRY is the property. POWERSHELL_TELEMETRY_OPTOUT is the environment variable name. The tests say the existing environment setting is kept unless someone set's DISABLE_TELEMETRY to 1. My main concern is silent upgrades, and this gives the correct behavior. Reading the setting in, would just make the logic more complex for no gain in functionality for the scenario I'm concerned about.

Travis Plunk (@TravisEz13) This appears to have made the Add 'Run with PowerShell 7' context menu for PowerShell files option disappear from the installer's GUI:

🎉v7.4.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes