Skip to content

Conversation

silverweed
Copy link
Contributor

Also change the failure in GenerateZeroPages from a R__LOG_FATAL to a ROOT::Result failure (we don't really need to abort the whole process if merging fails)

Checklist:

  • tested changes locally

@silverweed silverweed requested review from hahnjo and enirolf July 31, 2025 10:00
@silverweed silverweed self-assigned this Jul 31, 2025
@silverweed silverweed requested a review from jblomer as a code owner July 31, 2025 10:00
Copy link

github-actions bot commented Jul 31, 2025

Test Results

    21 files      21 suites   3d 3h 55m 54s ⏱️
 3 247 tests  3 244 ✅ 0 💤 3 ❌
66 475 runs  66 472 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit 0b91676.

♻️ This comment has been updated with latest results.

@silverweed silverweed force-pushed the ntuple_merge_streamer_test branch from 027e773 to bfb6c8f Compare August 4, 2025 08:31
@silverweed silverweed requested a review from jblomer August 6, 2025 08:35
Copy link
Member

@pcanal pcanal left a comment

Choose a reason for hiding this comment

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

Thanks.

Also change the failure in GenerateZeroPages from a R__LOG_FATAL to a
ROOT::Result failure
Also add [[nodiscard]] to RResultBase::ForwardError to prevent erroneous
use of R__FORWARD_ERROR
@silverweed silverweed force-pushed the ntuple_merge_streamer_test branch from bfb6c8f to 0b91676 Compare August 7, 2025 06:43
@silverweed silverweed added the clean build Ask CI to do non-incremental build on PR label Aug 7, 2025
@silverweed silverweed closed this Aug 7, 2025
@silverweed silverweed reopened this Aug 7, 2025
@silverweed silverweed merged commit 60dfe60 into root-project:master Aug 8, 2025
67 of 75 checks passed
@silverweed silverweed deleted the ntuple_merge_streamer_test branch August 8, 2025 06:44
Comment on lines +833 to +837
if (fIsInitialized)
for (const auto &field : changeset.fAddedFields)
if (field->GetStructure() == ENTupleStructure::kStreamer)
throw ROOT::RException(R__FAIL("a Model cannot be extended with Streamer fields"));

Copy link
Member

Choose a reason for hiding this comment

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

This change should probably also be tested outside the merger, in tree/ntuple/test/ntuple_modelext.cxx, as it's a general restriction...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean build Ask CI to do non-incremental build on PR in:RNTuple
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants