Skip to content

Conversation

@vogella
Copy link
Contributor

@vogella vogella commented Dec 12, 2025

This addresses the proposal by ptziegler in PR #3542 to split the removal of isCanceled checks.

This addresses the proposal by ptziegler in PR eclipse-platform#3542 to split the removal of isCanceled checks.
@vogella vogella requested a review from ptziegler December 12, 2025 10:05
@github-actions
Copy link
Contributor

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 21m 57s ⏱️ -58s
 8 242 tests ±0   7 993 ✅  - 1  248 💤 ±0  1 ❌ +1 
23 646 runs  ±0  22 854 ✅  - 1  791 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 7d16b28. ± Comparison against base commit e15e9bf.

Copy link
Contributor

@ptziegler ptziegler left a comment

Choose a reason for hiding this comment

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

The call of newChild() has been changed to split() as part of 'Bug 479879 - Adopt SubMonitor.split in platform UI'. For those classes specifically in c7cb5c4.

These methods throw an OperationCanceledException if the monitor has been canceled by the user. If there are any side-effects from throwing an exception, they would've become apparent by now.

I would even go so far and say that simply breaking out of the loop is an anti-pattern, because the program could now continue in a partially saved state, rather than properly handling the cancellation.

@ptziegler
Copy link
Contributor

The test case has been sporadically failing and is tracked via #3581. It should be irrelevant to this change.

@ptziegler ptziegler merged commit 26ddd0d into eclipse-platform:master Dec 14, 2025
20 of 22 checks passed
@ptziegler ptziegler added this to the 4.39 M1 milestone Dec 14, 2025
@vogella
Copy link
Contributor Author

vogella commented Dec 14, 2025

Thanks @ptziegler

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.

2 participants