Skip to content

Conversation

@ferdymercury
Copy link
Collaborator

This Pull request:

Changes or fixes:

Fixes https://its.cern.ch/jira/browse/ROOT-10705

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@ferdymercury ferdymercury requested a review from lmoneta as a code owner March 13, 2024 15:49
@phsft-bot
Copy link

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

@ferdymercury

This comment was marked as outdated.

@ferdymercury ferdymercury mentioned this pull request Mar 13, 2024
15 tasks
@ferdymercury ferdymercury changed the title [hist] fix x0 printEntries in thnBase [hist] fix x0 printEntries in THnSparse Mar 13, 2024
@ferdymercury ferdymercury changed the title [hist] fix x0 printEntries in THnSparse [hist] fix x0 printEntries in THnSparse/THnBase Mar 13, 2024
@github-actions
Copy link

github-actions bot commented Mar 13, 2024

Test Results

    12 files      12 suites   2d 2h 57m 28s ⏱️
 2 565 tests  2 565 ✅ 0 💤 0 ❌
29 036 runs  29 036 ✅ 0 💤 0 ❌

Results for commit 49cd6e3.

♻️ This comment has been updated with latest results.

@dpiparo
Copy link
Member

dpiparo commented Mar 14, 2024

Hi @ferdymercury , thanks a lot for this code. Do you think we can also add a test for it in root or roottest?

@ferdymercury
Copy link
Collaborator Author

You are welcome! Yes, sure, here it is: root-project/roottest#1086

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 !
A small suggestion, I would not use C functions like strchr but a TString and TString::Contains

@ferdymercury
Copy link
Collaborator Author

LGTM ! A small suggestion, I would not use C functions like strchr but a TString and TString::Contains

Thanks!
wrt strchr: It was just for consistency with the rest of the file.
There are 17x uses of strchr and I didn't feel like changing them all in this PR :)

@lmoneta lmoneta merged commit fb52a24 into root-project:master Mar 15, 2024
@ferdymercury ferdymercury deleted the thnbasex0 branch March 15, 2024 14:14
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.

4 participants