[ntuple] Clean up utility definitions #17359
Conversation
jblomer
force-pushed
the
ntuple-cleanup-utils
branch
from
d2d61ea to
8f0cb39
Compare
jblomer
changed the title
Test Results 17 files 17 suites 3d 20h 51m 17s ⏱️ For more details on these failures, see this check. Results for commit 0dff097. ♻️ This comment has been updated with latest results. |
jblomer
force-pushed
the
ntuple-cleanup-utils
branch
from
94af528 to
cfc6ab2
Compare
| /// TODO(jblomer): consider moving this to `RNTupleDescriptor` | ||
| struct RNTupleLocator { | ||
| class RNTupleLocator { | ||
| public: |
There was a problem hiding this comment.
Hm, what's the advantage of a class if we have getters and setters without further checks? Looking at the changes in this commit, a struct feels more ergonomic...
There was a problem hiding this comment.
It's to be more resilient to future changes. Maybe we want to rename a member, or add a check (e.g., if the type used in fPosition fits the locator type).
There was a problem hiding this comment.
Ok. There is still a forward declaration of struct RNTupleLocator; in RPageStorageFile.hxx that is now causing warnings...
jblomer
changed the title
jblomer
changed the title
jblomer
force-pushed
the
ntuple-cleanup-utils
branch
from
cfc6ab2 to
f12814c
Compare
jblomer
force-pushed
the
ntuple-cleanup-utils
branch
from
f12814c to
75be5ac
Compare
| /// TODO(jblomer): consider moving this to `RNTupleDescriptor` | ||
| struct RNTupleLocator { | ||
| class RNTupleLocator { | ||
| public: |
There was a problem hiding this comment.
Ok. There is still a forward declaration of struct RNTupleLocator; in RPageStorageFile.hxx that is now causing warnings...
jblomer
force-pushed
the
ntuple-cleanup-utils
branch
from
75be5ac to
0dff097
Compare
Preparation to move the common definitions from RNTupleUtil.h out of the experimental namespace. Some definitions are removed or moved to
ROOT::Internal. Some API improvements for the remaining public types.Part of the PR series to move an initial set of RNTuple APIs out of the experimental namespace.