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

remove redundant isDisposing pattern in StressTest #6082

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

Conversation

SimonCropp
Copy link
Contributor

@SimonCropp SimonCropp commented Jan 22, 2025

Changes

Since no unmanaged resources are used and no finalizers to be suppressed

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@SimonCropp SimonCropp requested a review from a team as a code owner January 22, 2025 03:38
@github-actions github-actions bot added perf Performance related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package labels Jan 22, 2025
@SimonCropp
Copy link
Contributor Author

bump

@@ -41,15 +47,5 @@ protected override void RunWorkItemInParallel()
recallReasonDescription: "due to a possible health risk from Listeria monocytogenes",
companyName: "Contoso Fresh Vegetables, Inc.");
}

protected override void Dispose(bool isDisposing)
Copy link
Contributor

Choose a reason for hiding this comment

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

This follows the guidance from .NET - https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose. Is there a strong reason why these test projects need a change? They are executed on an as-needed basis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that guidance is for unmanaged resources. re read the first paragraph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as for “a strong reason”. removing redundant code should be reason enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf Performance related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants