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

[Minuit2] Revert recent changes to virtual function signature #16491

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

guitargeek
Copy link
Contributor

This reverts part of 63636f6 from a few days ago, which also changed the interface of virtual functions, which was not the intention and breaks user code.

This reverts part of 63636f6 from a few days ago, which also changed
the interface of virtual functions, which was not the intention and
breaks user code.
@guitargeek
Copy link
Contributor Author

@smuzaffar, FYI. Sorry for accidentally breaking the code! My other PR was done with find and replace, and I didn't consider this case of virtual functions where changing the signature actually breaks user code!

I will also open a PR to CMSSW to make things right. Because there is indeed also a PR with breaking changes that were intended and that will further break cmssw: #16443

Other than that PR, no breaking changes are planned for Minuit2.

Copy link

github-actions bot commented Sep 20, 2024

Test Results

    13 files      13 suites   3d 1h 38m 25s ⏱️
 2 696 tests  2 696 ✅ 0 💤 0 ❌
32 940 runs  32 940 ✅ 0 💤 0 ❌

Results for commit d9b0461.

♻️ This comment has been updated with latest results.

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!
Thank you for fixing this

@guitargeek
Copy link
Contributor Author

@smuzaffar, @iarspider, would now be a good moment to merge this? If not, please let me know when. Thanks!

@smuzaffar
Copy link
Contributor

yes , it can go in. I will any way test it with your cmssw PR cms-sw/cmssw#46079 now

@guitargeek
Copy link
Contributor Author

Ok, thanks!

@guitargeek guitargeek merged commit d8458dd into root-project:master Oct 3, 2024
17 of 19 checks passed
@guitargeek guitargeek deleted the minuit2_span_followup branch November 30, 2024 12:33
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