Skip to content
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

Updated Initialize.cs to show modern dotnet4+ versions #6128

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

jeremy-farrance
Copy link
Contributor

@jeremy-farrance jeremy-farrance commented Sep 2, 2024

Fixes #6127

Summary

Quick update so that the .NET Framework version shown (e.g. in the PersonaBar) is accurate up to v4.8.1.

Also updated the code style (no more else) making it easy to added new versions in the future.

Reference: DotNet Versions via Code

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the build failed...

Removed untested line of code, restored extra newline after close brace `}`. Build works again for me locally.
@jeremy-farrance
Copy link
Contributor Author

jeremy-farrance commented Sep 2, 2024

Except the build failed...

Sorry about that. Fixed and works for me now (local build).

@valadas
Copy link
Contributor

valadas commented Sep 2, 2024

There is still 2:

image

@jeremy-farrance
Copy link
Contributor Author

There is still 2:

Are you messing with me? I just fixed the opposite above, "SA1513 - Closing brace should be followed by blank line"

I tried this in both VS Code and VS 2022. The only way not to get one of the two errors is ...

Oh, so both editors are just inserting whitespace up to the indented cursor. I am supposed to know to spot that and fix it?

Okay, sorry, lesson learned. Follow the rules even if you can't see the difference. I think this will fix it, at least I get neither error in VS 2022 when I force the blank lines.

Replaced whitespace with blank lines to avoid SA1028 and SA1513 simultaneously. :)
@bdukes bdukes added this to the 9.13.5 milestone Sep 3, 2024
{
version = "4.8.1";
}

if (release >= 528040)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? Doesn't this need to be in descending order? If release is 533320, that should mean 4.8.1, but then it will also be greater than 528040 and 461808, so won't version end up being set to "4.7.2"? Maybe I'm getting this mixed up in my head, but it doesn't read to me like it should work correctly without an else before each if.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, it doesn't. It was lazy-converted from using return "4.8";. I'll switch this pull to draft until I can get this done properly. Thanks!

@jeremy-farrance jeremy-farrance marked this pull request as draft September 3, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DotNet Framework Version is not Accurate
3 participants