Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move "Light (Preview)" Theme to the "Light" Theme #2549

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

BeckerWdf
Copy link
Contributor

  • Copies over all changes done in the "Light (Preview)" theme to the "Light" theme. So the changes done in the preview theme are now the default.

  • Delete the "Light (Preview)" theme as it's no longer needed.

Also:

  • Simplify the plugin.properties file to remove duplicated strings.

See: #2114

@BeckerWdf BeckerWdf added this to the 4.35 M1 milestone Nov 27, 2024
@BeckerWdf
Copy link
Contributor Author

As discussed this PR does bring the changes done in the light preview theme to the "default" light theme (on all 3 SWT platforms). This is now done early in the 4.35 cycle to get further feedback and to have enough time to fix arising issues.

@eclipse-platform-bot
Copy link
Contributor

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

bundles/org.eclipse.ui.themes/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 92442dbc5cbd758a6d87a60cea4ae74d1996c12d Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Wed, 27 Nov 2024 13:48:21 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
index 0338462b26..59abbfd238 100644
--- a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.ui.themes;singleton:=true
-Bundle-Version: 1.2.2600.qualifier
+Bundle-Version: 1.2.2700.qualifier
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.e4.ui.css.swt.theme
-- 
2.47.0

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

@BeckerWdf BeckerWdf self-assigned this Nov 27, 2024
@BeckerWdf BeckerWdf requested a review from sratz November 27, 2024 14:04
@vogella
Copy link
Contributor

vogella commented Nov 27, 2024

Looks great, lets merge this early in this release cycle so that we iron issue out during this release cycle.

Thanks a bunch @BeckerWdf .

Copy link
Contributor

github-actions bot commented Nov 27, 2024

Test Results

 1 214 files   -   300   1 214 suites   - 300   1h 24m 9s ⏱️ - 20m 33s
 7 726 tests ±    0   7 494 ✅  -     3  231 💤 + 2  1 ❌ +1 
16 226 runs   - 6 267  15 712 ✅  - 6 233  513 💤  - 35  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 6dec95a. ± Comparison against base commit 54adc6a.

This pull request skips 2 tests.
org.eclipse.jface.text.tests.contentassist.ContextInformationTest ‑ testContextInfo_hide_focusOut
org.eclipse.urischeme.internal.registration.TestUnitWinRegistry ‑ testWinRegistry

♻️ This comment has been updated with latest results.

- Copies over all changes done in the "Light (Preview)" theme to the
"Light" theme. So the changes done in the preview theme are now the
default.

- Delete the "Light (Preview)" theme as it's no longer needed.

Also:
- Simplify the plugin.properties file to remove duplicated strings.

See: eclipse-platform#2114
@BeckerWdf
Copy link
Contributor Author

CI Build says:

Failed to execute goal org.eclipse.tycho:tycho-surefire-plugin:4.0.10:test (default-test) on project org.eclipse.ui.tests: An unexpected error occurred while launching the test runtime (process returned error code 13).

What's wrong here?

@mickaelistria
Copy link
Contributor

If you look at the log and scroll up to where org.eclipse.ui.tests is built and tests are run, you'll see

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "Active Thread: Equinox Container: dbdc8da7-d7d6-4bc4-bb10-858d54a3575f"
Exception in thread "WorkbenchTestable" Exception in thread "Worker-6" java.lang.OutOfMemoryError: Java heap space
java.lang.OutOfMemoryError: Java heap space
An error has occurred. See the log file
/home/jenkins/agent/workspace/eclipse.platform.ui_PR-2549/tests/org.eclipse.ui.tests/target/work/data/.metadata/.log.

@BeckerWdf
Copy link
Contributor Author

If you look at the log and scroll up to where org.eclipse.ui.tests is built and tests are run, you'll see

Exception: java.lang.OutOfMemoryError thrown from the UncaughtExceptionHandler in thread "Active Thread: Equinox Container: dbdc8da7-d7d6-4bc4-bb10-858d54a3575f"
Exception in thread "WorkbenchTestable" Exception in thread "Worker-6" java.lang.OutOfMemoryError: Java heap space
java.lang.OutOfMemoryError: Java heap space
An error has occurred. See the log file
/home/jenkins/agent/workspace/eclipse.platform.ui_PR-2549/tests/org.eclipse.ui.tests/target/work/data/.metadata/.log.

So this is unrelated. I also don't expect that theme changes would cause failing tests. So let's merge this.

@BeckerWdf BeckerWdf merged commit 5930e6b into eclipse-platform:master Nov 28, 2024
11 of 14 checks passed
@BeckerWdf BeckerWdf deleted the light_theme branch November 28, 2024 11:45
@mickaelistria
Copy link
Contributor

Did you see any other build showing an OutOfMemery error before merging this patch?

@mickaelistria
Copy link
Contributor

I tried a revert (#2559) to see whether the OOM was reproduced without the change, and it is reproduceable, so this particular change doesn't seem to blame.

@iloveeclipse
Copy link
Member

OOM's seem to started recently. I've blamed infra but @HannesWell meant there were no changes. It would be nice if someone could investigate, I will be back to PC on Monday.

@BeckerWdf BeckerWdf added the noteworthy Noteworthy feature label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noteworthy Noteworthy feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants