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

Fix ownership for end method in StartedTestRun #25

Closed
wants to merge 1 commit into from

Conversation

njordr
Copy link
Collaborator

@njordr njordr commented Dec 12, 2024

Fix the end method that takes ownership of self without any needs

@njordr njordr requested a review from mimir-d December 12, 2024 11:06
Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.5%. Comparing base (081cd2d) to head (07591a7).

Additional details and impacted files

@mimir-d
Copy link
Contributor

mimir-d commented Jan 7, 2025

takes ownership of self without any needs

this is incorrect; the "need" is semantic. All these end methods (in StartedTestRun, StartedTestStep) consume self in order to make it impossible to output new messages after the test run/step has ended.

in the example https://github.com/opencomputeproject/ocp-diag-core-rust/blob/dev/examples/simple_no_scopes.rs#L27, it makes it such that writing something like step.add_diagnosis() after line 27 would not compile.

@njordr njordr closed this Jan 8, 2025
@mimir-d mimir-d deleted the bug/fix_started_test_run_ownership branch January 9, 2025 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants