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

[RF] Change return type of RooRealVar::format() to std::string #16954

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

guitargeek
Copy link
Contributor

Returning a TString by raw pointer that the caller needs to delete is not memory safe and an outdated pattern, which merits a breaking change documented in the 6.36 release notes.

Copy link

github-actions bot commented Nov 15, 2024

Test Results

    17 files      17 suites   4d 3h 8m 11s ⏱️
 2 694 tests  2 694 ✅ 0 💤 0 ❌
44 144 runs  44 144 ✅ 0 💤 0 ❌

Results for commit a936fa3.

♻️ This comment has been updated with latest results.

@guitargeek guitargeek requested a review from aaronj0 November 27, 2024 12:58
Returning a TString by raw pointer that the caller needs to delete is
not memory safe and an outdated pattern, which merits a breaking change
documented in the 6.36 release notes.
All tests are parametrized for the evaluation backend, but the parameter
was not used for all likelihoods that were created in the tests.

This commit fixes that, such that each test now consistently uses one
evaluation backend.
Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM!
I agree is better to have this change!

@guitargeek guitargeek merged commit e7cc7df into root-project:master Jan 10, 2025
19 of 20 checks passed
@guitargeek guitargeek deleted the tstring_ptr branch January 10, 2025 18:41
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.

2 participants