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

Clean Code for bundles/org.eclipse.osgi #796

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eclipse-equinox-bot
Copy link
Contributor

@eclipse-equinox-bot eclipse-equinox-bot commented Jan 27, 2025

The following cleanups where applied:

  • Add final modifier to private fields
  • Add missing '@Deprecated' annotations
  • Add missing '@Override' annotations
  • Add missing '@Override' annotations to implementations of interface methods
  • Convert control statement bodies to block
  • Make inner classes static where possible
  • Replace deprecated calls with inlined content where possible

Copy link

github-actions bot commented Jan 27, 2025

Test Results

  669 files  ±0    669 suites  ±0   1h 16m 23s ⏱️ +5s
2 223 tests ±0  2 176 ✅ ±0   47 💤 ±0  0 ❌ ±0 
6 813 runs  ±0  6 670 ✅ ±0  143 💤 ±0  0 ❌ ±0 

Results for commit 6b5efa6. ± Comparison against base commit 772515b.

♻️ This comment has been updated with latest results.

Copy link
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Cleanup produces a compile error, need to investigate.

@laeubi
Copy link
Member

laeubi commented Jan 27, 2025

I think this is similar to

I need to adjust Tycho to pass in the configured toolchain JDKs so we get a real java 8 JVM most likely.

@laeubi laeubi marked this pull request as draft January 27, 2025 11:48
@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch from 3779d22 to 48abd4a Compare January 28, 2025 03:36
@laeubi
Copy link
Member

laeubi commented Jan 28, 2025

@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch 2 times, most recently from 62d8981 to 1bbcc81 Compare January 30, 2025 03:33
@laeubi
Copy link
Member

laeubi commented Jan 30, 2025

@tjwatson this also cleanups the felix embedded code, we actually have two options here I think

  1. I implement some kind of exclusion rules in tycho so we can skip some source path
  2. We apply them as is to the codebase

@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch from 1bbcc81 to 657162c Compare January 30, 2025 08:16
@laeubi laeubi marked this pull request as ready for review January 30, 2025 10:32
Copy link
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Also working here now, but waiting for feedback from @tjwatson

Copy link
Contributor

@tjwatson tjwatson left a comment

Choose a reason for hiding this comment

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

The changes to org.eclipse packages look fine. We should not change anything to org.osgi or org.apache source. That should be done in the respective projects. Especially for the org.osgi APIs

@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch 3 times, most recently from 0ba013e to 8855a87 Compare February 6, 2025 03:36
@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch 2 times, most recently from 11f980f to f0b3ccb Compare February 14, 2025 03:25
@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch 3 times, most recently from fc3d7e0 to 8342deb Compare March 5, 2025 03:31
@eclipse-equinox-bot
Copy link
Contributor Author

eclipse-equinox-bot commented Mar 5, 2025

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.osgi/META-INF/MANIFEST.MF
bundles/org.eclipse.osgi/pom.xml
bundles/org.eclipse.osgi/supplement/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 a3c2a1ccd4864ef9b0ed840abcbc27a28fd4e241 Mon Sep 17 00:00:00 2001
From: Eclipse Equinox Bot <equinox-bot@eclipse.org>
Date: Tue, 11 Mar 2025 03:51:15 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
index 0ec46ec97..17ecf574f 100644
--- a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF
@@ -107,7 +107,7 @@ Bundle-Activator: org.eclipse.osgi.internal.framework.SystemBundleActivator
 Bundle-Description: %systemBundle
 Bundle-Copyright: %copyright
 Bundle-Vendor: %eclipse.org
-Bundle-Version: 3.23.0.qualifier
+Bundle-Version: 3.23.100.qualifier
 Bundle-Localization: systembundle
 Bundle-DocUrl: http://www.eclipse.org
 Eclipse-ExtensibleAPI: true
diff --git a/bundles/org.eclipse.osgi/pom.xml b/bundles/org.eclipse.osgi/pom.xml
index 0b0ea68d6..63105001f 100644
--- a/bundles/org.eclipse.osgi/pom.xml
+++ b/bundles/org.eclipse.osgi/pom.xml
@@ -19,7 +19,7 @@
 </parent>
   <groupId>org.eclipse.platform</groupId>
   <artifactId>org.eclipse.osgi</artifactId>
-  <version>3.23.0-SNAPSHOT</version>
+  <version>3.23.100-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
   <properties>
 	  <!-- The actual TCKs are executed in the org.eclipse.osgi.tck module because of reference to other service implementations -->
diff --git a/bundles/org.eclipse.osgi/supplement/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi/supplement/META-INF/MANIFEST.MF
index 6286ac09b..89e8ef752 100644
--- a/bundles/org.eclipse.osgi/supplement/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.osgi/supplement/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.equinox.supplement
-Bundle-Version: 1.12.0.qualifier
+Bundle-Version: 1.12.100.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.equinox.log;version="1.1",
-- 
2.48.1

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

@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch from 963bf48 to 3906bc9 Compare March 5, 2025 06:26
@laeubi
Copy link
Member

laeubi commented Mar 5, 2025

@tjwatson I now added the excludes, the rest looks fine for me, could you probably also review and approve?

@laeubi laeubi requested a review from tjwatson March 5, 2025 07:24
@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch 5 times, most recently from fd94d85 to 7f36c31 Compare March 9, 2025 03:33
@@ -1291,9 +1345,10 @@ private static void updateSplash(Semaphore semaphore, StartupEventListener liste
}
}
// can we acquire the semaphore yet?
if (semaphore.tryAcquire(50, TimeUnit.MILLISECONDS))
if (semaphore.tryAcquire(50, TimeUnit.MILLISECONDS)) {
break; // done
// else still working, spin another update
Copy link
Member

Choose a reason for hiding this comment

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

/* xxx */
int index = string.indexOf(substr2, pos);
if (index == -1) {
return false;
}
pos = index + substr2.length();
if (i + 2 < size) // if there are more
if (i + 2 < size) { // if there are more
Copy link
Member

Choose a reason for hiding this comment

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

@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch from 4ed30a7 to 40791c4 Compare March 10, 2025 03:37
@eclipse-equinox-bot eclipse-equinox-bot force-pushed the clean-code/bundles/org.eclipse.osgi branch from b046838 to da34f10 Compare March 11, 2025 03:46
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.

3 participants