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

hosting windows service helper enhancement #93771

Merged
merged 3 commits into from
Oct 31, 2023
Merged

Conversation

WeihanLi
Copy link
Contributor

fixes #93613

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 20, 2023
@ghost
Copy link

ghost commented Oct 20, 2023

Tagging subscribers to this area: @dotnet/area-extensions-hosting
See info in area-owners.md if you want to be subscribed.

Issue Details

fixes #93613

Author: WeihanLi
Assignees: -
Labels:

area-Extensions-Hosting

Milestone: -

@huoyaoyuan
Copy link
Member

I wonder if we can convert IsOSPlatform to OperatingSystem.IsWindows() and pull the check out. Will this help for trimming?

@buyaa-n
Copy link
Member

buyaa-n commented Oct 30, 2023

I wonder if we can convert IsOSPlatform to OperatingSystem.IsWindows() and pull the check out. Will this help for trimming?

Not sure how it affects trimming but, using OperatingSystem.IsWindows() on .NET 5+ build is preferable. @WeihanLi could you add an #elif case for that? Thank you!

            if (
#if NETFRAMEWORK
                Environment.OSVersion.Platform != PlatformID.Win32NT
#elif NET5_0_OR_GREATER
                !OperatingSystem.IsWindows()
#else
                !RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
#endif
                )
            {
                return false;
            }

using OperatingSystem for net 5.0 or greater
WeihanLi added a commit to WeihanLi/runtime that referenced this pull request Oct 30, 2023
Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@buyaa-n buyaa-n merged commit 18cb172 into dotnet:main Oct 31, 2023
109 checks passed
@WeihanLi WeihanLi deleted the patch-1 branch November 1, 2023 01:47
stephentoub pushed a commit that referenced this pull request Nov 1, 2023
* Update SystemdHelpers.cs

align code style with #93771

* Update SystemdHelpers.cs

revert OperationSystem condition change

* remove partial on SystemdHelpers
@ghost ghost locked as resolved and limited conversation to collaborators Dec 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Hosting community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance Issue with IsWindowsService due to Repeated Process Lookups
3 participants