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

#5962 Change to early return of OS instead of throwing #5963

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

juliankock
Copy link

@juliankock juliankock commented Feb 24, 2025

This will make sure that the executed code only works in scenarios that are supported, which is Windows and Linux. This is particularly important for teams working on different operating systems, and running the code without containerization.

Addresses: #5962

Microsoft Reviewers: Open in CodeFlow

This will make sure that the executed code only works in scenarios that
are supported, which is Windows and Linux. This is particularly
important for teams working on different operating systems, and running
the code without containerization.
@juliankock juliankock requested a review from a team as a code owner February 24, 2025 13:36
@evgenyfedorov2 evgenyfedorov2 added waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. * NO MERGE * Do not merge this PR as long as this label is present. labels Feb 24, 2025
@dotnet-policy-service dotnet-policy-service bot removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Mar 3, 2025
@RussKie RussKie marked this pull request as draft March 4, 2025 06:23
@vidarasberg
Copy link

I am looking forward to this change as I too am on a mac and am experiencing the same issue.

Why is this only a draft? It looks good to me :)

@juliankock juliankock marked this pull request as ready for review March 6, 2025 20:23
@juliankock
Copy link
Author

@evgenyfedorov2 It passes tests now. Is there anything we would like more?

@@ -63,13 +63,17 @@ private static IServiceCollection AddResourceMonitoringInternal(
this IServiceCollection services,
Copy link
Contributor

@evgenyfedorov2 evgenyfedorov2 Mar 7, 2025

Choose a reason for hiding this comment

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

This method can now be fully covered by unit tests, please remove the comment and ExcludeFromCodeCoverage attribute

@evgenyfedorov2
Copy link
Contributor

@evgenyfedorov2 It passes tests now. Is there anything we would like more?

Yeah, please just add more tests, I left a comment.

@evgenyfedorov2 evgenyfedorov2 removed the * NO MERGE * Do not merge this PR as long as this label is present. label Mar 7, 2025
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.

3 participants