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

Avoid unnecessary HealthStatus comparison #58176

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

drewnoakes
Copy link
Member

Avoid unnecessary HealthStatus comparison

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.
    • There's no open issue for this. It's a drive-by micro-optimisation.

Description

It's only necessary to check currentValue == HealthStatus.Unhealthy when currentValue changes. During the first iteration, it is known to be Healthy.

It's only necessary to check `currentValue == HealthStatus.Unhealthy` when `currentValue` changes. During the first iteration, it is known to be `Healthy`.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Oct 1, 2024
return currentValue;
if (currentValue == HealthStatus.Unhealthy)
{
// Game over, man! Game over!
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it worth considering replacing this comment with something more appropriate?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think I'd encourage people to write comments in this tone/register, but it wasn't introduced by this change and doesn't seem specifically inappropriate, so I'm okay with leaving it for now.

@amcasey amcasey merged commit 3bc8d5b into main Oct 3, 2024
27 checks passed
@amcasey amcasey deleted the drnoakes/heath-status-comparison branch October 3, 2024 20:42
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants