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

Always sort BREE header values in ascending Java version order #1324

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Jul 6, 2024

Remove the ability to manually move required execution environments in the Manifest Editor. Instead always sort the values in ascending Java version order.

From a technical perspective the order of the values of the Bundle-RequiredExecutionEnvironment header is irrelevant, similar to for example the Import-Package header. Removing the buttons to reorder the values simplifies the UI.

Values of existing Plug-in projects are displayed in sorted order regardless of if they are actually sorted in the MANIFEST.MF or not. On the next write all values will be written in sorted order.

With this change applied the EE section now looks like this:

grafik

Copy link

github-actions bot commented Jul 6, 2024

Test Results

   291 files  +   85     291 suites  +85   56m 9s ⏱️ + 20m 39s
 3 580 tests ±    0   3 504 ✅ +    1   76 💤  -  1  0 ❌ ±0 
11 037 runs  +3 631  10 806 ✅ +3 555  231 💤 +76  0 ❌ ±0 

Results for commit 5a45f24. ± Comparison against base commit 7d8b8f2.

♻️ This comment has been updated with latest results.

@HannesWell
Copy link
Member Author

@laeubi, @merks what do you think about this? Any objections?

As a side note: I just created eclipse-equinox/equinox#655 as an attempt to make the order of some old Java version as expected. But since it only affects old versions I don't consider it a prerequisite.

@HannesWell HannesWell force-pushed the no-bree-move branch 2 times, most recently from e69ca13 to ca1196a Compare July 7, 2024 11:47
Comment on lines 111 to 123
return switch (ee.getId()) {
// The EE's handled explicitly have unexpected java targets.
// See https://github.com/eclipse-equinox/equinox/pull/655
case "J2SE-1.2" -> 1.2; //$NON-NLS-1$
case "J2SE-1.3" -> 1.3; //$NON-NLS-1$
case "J2SE-1.4" -> 1.4; //$NON-NLS-1$
default -> {
Properties properties = ee.getProfileProperties();
Object target = properties != null //
? properties.get(JavaCore.COMPILER_CODEGEN_TARGET_PLATFORM)
: null;
yield target instanceof String version ? Double.parseDouble(version) : 0;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Just for clarification: This part could probably be simplified to only the default case if eclipse-equinox/equinox#655 were available.

Remove the ability to manually move required execution environments in
the Manifest Editor. Instead always sort the values in ascending Java
version order.

From a technical perspective the order of the values of the
'Bundle-RequiredExecutionEnvironment' header is irrelevant, similar to
for example the 'Import-Package' header.
Removing the buttons to reorder the values simplifies the UI.

Values of existing Plug-in projects are displayed in sorted order
regardless of if they are actually sorted in the MANIFEST.MF or not. On
the next write all values will be written in sorted order.
@HannesWell
Copy link
Member Author

I'm going to submit this now to unblock #1285, but still hope that eclipse-equinox/equinox#655 can be leveraged later.

@HannesWell HannesWell merged commit ece4417 into eclipse-pde:master Jul 9, 2024
17 checks passed
@HannesWell HannesWell deleted the no-bree-move branch July 9, 2024 21:32
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.

1 participant