eclipse.platform icon indicating copy to clipboard operation
eclipse.platform copied to clipboard

Fix incorrect SubMonitor usage patterns

Open vogella opened this issue 2 months ago • 2 comments

This commit addresses several SubMonitor anti-patterns identified throughout the codebase, based on best practices from the Eclipse article on Progress Monitors.

Changes made:

  1. Remove unnecessary null checks before SubMonitor.convert()

    • SubMonitor.convert() can handle null monitors
    • Fixed in team Policy.java files (core and ui)
  2. Remove unnecessary SubMonitor.done() calls

    • SubMonitor.done() is a no-op and should not be called
    • Removed 27 instances across resources and team bundles:
      • org.eclipse.core.filesystem: LocalFile.java (2 calls)
      • org.eclipse.core.resources: File.java (6), Folder.java (1), Project.java (3), SaveManager.java (4), Workspace.java (7) * org.eclipse.compare.core: RangeComparatorLCS.java (1) * org.eclipse.compare: AddFromHistoryAction.java (1) * org.eclipse.team.core: BundleImporterDelegate.java (1), Subscriber.java (1)
  3. Remove redundant cancellation checks before split()

    • SubMonitor.split() already checks for cancellation
    • Fixed in AutoBuildJob.java

Reference: https://www.eclipse.org/articles/Article-Progress-Monitors/article.html

vogella avatar Nov 14 '25 01:11 vogella

This pull request changes some projects for the first time in this development cycle. Therefore the following files need a version increment:

team/bundles/org.eclipse.jsch.core/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From 0b4558ece14037bf6a653283c7ae3d9ef1279f4c Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Fri, 14 Nov 2025 08:10:08 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/team/bundles/org.eclipse.jsch.core/META-INF/MANIFEST.MF b/team/bundles/org.eclipse.jsch.core/META-INF/MANIFEST.MF
index 82861fbee2..83de036ad6 100644
--- a/team/bundles/org.eclipse.jsch.core/META-INF/MANIFEST.MF
+++ b/team/bundles/org.eclipse.jsch.core/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jsch.core;singleton:=true
-Bundle-Version: 1.5.700.qualifier
+Bundle-Version: 1.5.800.qualifier
 Bundle-Activator: org.eclipse.jsch.internal.core.JSchCorePlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
-- 
2.51.2

Further information are available in Common Build Issues - Missing version increments.

eclipse-platform-bot avatar Nov 14 '25 01:11 eclipse-platform-bot

Test Results

 1 872 files   -  81   1 872 suites   - 81   1h 22m 18s ⏱️ - 1m 22s  4 591 tests  - 153   3 871 ✅  -   849   24 💤 ± 0    694 ❌ +  694  2 🔥 +2  13 773 runs   - 459  11 826 ✅  - 2 224  165 💤  - 17  1 778 ❌ +1 778  4 🔥 +4 

For more details on these failures and errors, see this check.

Results for commit e39b98b0. ± Comparison against base commit 3e8e40e7.

This pull request removes 153 tests.
AllCompareTests AsyncExecTests ‑ testCancelOnRequeue
AllCompareTests AsyncExecTests ‑ testQueueAdd
AllCompareTests AsyncExecTests ‑ testWorker
AllCompareTests CompareFileRevisionEditorInputTest ‑ testPrepareCompareInputWithNonLocalResourceTypedElements
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptorForTextType_StreamAccessor
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptor_TextType_NotStreamAccessor
AllCompareTests CompareUIPluginTest ‑ testFindContentViewerDescriptor_UnknownType
AllCompareTests ContentMergeViewerTest ‑ testFFFX
AllCompareTests ContentMergeViewerTest ‑ testFFTX
AllCompareTests ContentMergeViewerTest ‑ testFFXF
…

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Nov 14 '25 01:11 github-actions[bot]