search

eclipse-equinox / equinox.bundles

Eclipse Public License 2.0
8 stars 16 forks source link

Remove load of unsupported legacy preference files #15

Closed HannesWell closed 2 years ago

HannesWell commented 2 years ago

As suggested by @vogella in PR #13 this PR suggests to remove the code to load legacy Eclipse 2.1 preference files that are not supported for a while. I cannot tell since when those files were considered legacy but in the beginning of the git history of the InstancePreferences class 16 years ago they were already considered legacy. And I don't expect anybody to update from a more than 16year old Eclipse to a current one and to expect that it will work out of the box. But somebody with a greater overview should make the decision to submit this or not.

vogella commented 2 years ago

Thanks Hannes, no need to keep legacy code around. I would have expected that API tooling complains here, but as it does not, I merge it.

Thanks again, Hannes.

tjwatson commented 2 years ago

Is this really about old 2.0 plugins or simply some preferences format at version 2.1 that is disjoint from Eclipse 2.0 release? I honestly don't know, but worry not enough thought was put into removing this.

HannesWell commented 2 years ago

Is this really about old 2.0 plugins or simply some preferences format at version 2.1 that is disjoint from Eclipse 2.0 release? I honestly don't know, but worry not enough thought was put into removing this.

Me neither. I can only tell that these preferences were already considered legacy when this class was initially commited into this repository in 2005: https://github.com/eclipse-equinox/equinox.bundles/blob/45009ada3d70007a38c2b01760ee3990156cf0f3/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/InstancePreferences.java

From this I would expect that it won't affect many people, but as I said, I don't have a good overview in this area.

iloveeclipse commented 2 years ago

There is a new test fail caused by this change, tests must be updated too.

https://download.eclipse.org/eclipse/downloads/drops4/I20220414-1800/testresults/html/org.eclipse.core.tests.runtime_ep424I-unit-cen64-gtk3-java11_linux.gtk.x86_64_11.html

org.eclipse.core.tests.internal.preferences.EclipsePreferencesTest | testLegacy | Failure | 
3.0 expected:<value.1649981901638-0.3893493642818254> but was:<null>junit.framework.ComparisonFailure: 3.0 expected:<value.1649981901638-0.3893493642818254> but was:<null>at junit.framework.Assert.assertEquals(Assert.java:100)at junit.framework.TestCase.assertEquals(TestCase.java:253)at org.eclipse.core.tests.internal.preferences.EclipsePreferencesTest.testLegacy(EclipsePreferencesTest.java:1091)at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
HannesWell commented 2 years ago

Thank you Andrey for fixing the test!

The comment of the removed test contains a Bug reference: Bug 56020 Although it does not provide much more context it says that the corresponding preferences format was already considered legacy in 2004. Without knowing more I tend to say that after 18years a legacy preference format does not need to be supported anymore.