dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
739 stars 1.57k forks source link

Environment.OSVersion has misleading notes on 8/10 compatibility #3605

Open ocdtrekkie opened 6 years ago

ocdtrekkie commented 6 years ago

I have a headless machine which I want to report its Windows version so I know if the machine is up to date, and Environment.OSVersion has been very frustrating. After learning about everything I've learned, it still doesn't do what I need, because I need the full revision number. (i.e. 10.0.16299.192) and OSVersion insists on a .0 at the end. I'm using a WMI call instead. But anyways, Docs bug:

https://docs.microsoft.com/en-us/dotnet/api/system.environment.osversion?view=netframework-4.7.1

Both the "Important" and "Note" sections here simply state that from now on, OSVersion reports Windows 8 and above as Windows 6.2, with no qualifying details. This is wrong. I realized this due to reading the High DPI for Windows Forms documentation here: https://docs.microsoft.com/en-us/dotnet/framework/winforms/high-dpi-support-in-windows-forms

The correct information can only be found by digging into the "Operating System Version" link, which is an old MSDN page, and then noticing the link about "Targeting your application for Windows" at the bottom of that page. Nothing on System.Environment.OSVersion's Docs page would lead me to realize this.

Ideally, the Important/Note segments, which are somewhat redundant, should be collapsed into a single box which summarizes the default behavior as well as links directly to details on how to specify the supportedOS: https://msdn.microsoft.com/en-us/library/windows/desktop/dn481241.aspx

mairaw commented 6 years ago

Thanks for your feedback @ocdtrekkie. We'll investigate. @karelz who owns System.Environment that could help here too?

mairaw commented 6 years ago

I just tested this on .NET Core and .NET Framework. It seems that .NET Framework does return the correct value when the application manifest targets the OS, but Core doesn't seem to do that. If the application is not manifested to the target OS, then the comments on the current page applies.

We should probably add the information from here too: https://github.com/dotnet/platform-compat/blob/master/docs/DE0009.md

/cc @pjanotti @terrajobst any comments?

karelz commented 6 years ago

The type is in System.Runtime.Extensions - @joperezr (per area owners page)

pjanotti commented 6 years ago

In the context of CoreFX it seems reasonable to continue with current recommendation of DE0009. https://github.com/dotnet/platform-compat/issues/58 has the reasoning for that.

ocdtrekkie commented 6 years ago

As noted in my case, I check the version to see if my computer is up to date, since the computer is rarely online and cannot necessarily check for itself. (Right now, I know my home PC is on 15063.909, but my car PC, through my .NET app, says it's on 15063.786, ergo, I know it's out of date!)

I totally understand the desire for people not to use it for compatibility checks (given I have software at work I have to convince is running on a Vista machine to get it to install on Windows 10), though people who want to do that can do what I ended up doing here: a WMI call to grab all of the version info I wanted from the registry. (I find some personal irony in that, of course, it's recommended not to check the OSVersion for compatibility, but the High DPI in Windows Forms article, written a mere year ago, recommends doing just that.)

Anyways, this issue file was more meant to be a docs issue more than an opinion on .NET's functionality itself: The note and remarks here on Docs are from when Windows 8 and 8.1 were current, don't mention the supportedOs option explained in the High DPI article, and if this is different between .NET Core and .NET Framework, the remarks on this page should probably be updated to communicate that as well.

pjanotti commented 6 years ago

Thanks for the detailed feedback @ocdtrekkie we do appreciate it.

The docs definitely should be updated, the rule recommendation on the analyzer mentioned above is based that we see more cases of misuse then good use of the API. So let's update the docs but keep the current recommendation on the analyzer (it links to the doc for more info anyway).

mairaw commented 6 years ago

For sure, we need to improve those docs @ocdtrekkie since the information there is not correct and not complete. I was just trying to confirm if there was anything else I was missing.

@rpetrusha @merriemcgaw should we also update the high DPI article to show how to check for versions using RuntimeInformation.IsOSPlatform(OSPlatform) on newer apps?

This issue also made me realize we don't have good docs on the application manifest file. I'll create new issues if we deem that these other tasks should be done and keep this one focused on the API docs.