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

Get rid of deprecated org.eclipse.core.runtime.Preferences in platform code #497

Open
laeubi opened this issue Jun 14, 2023 · 14 comments
Open
Labels

Comments

@laeubi
Copy link
Contributor

laeubi commented Jun 14, 2023

org.eclipse.core.runtime.Preferences is deprecated since 2014, but there are still as of today 46 matches.

It would be good to get the reference count down her and seems a good first issue for contributors that want to get in touch with eclipse code or anyone interested in cleaning up things.

As there was a quite large attempt to "move" code to Java 17 it seems to have issues for deprecated references is even worth more.

@alex2145
Copy link
Contributor

If I understood correctly, you want to remove some references to the deprecated class org.eclipse.core.runtime.Preferences from the runtime module.

This will break all the plugins that use deprecated methods like Plugin.getPluginPreferences().

@laeubi
Copy link
Contributor Author

laeubi commented Jun 15, 2023

If we remove/replace the reference how could it break things?

@alex2145
Copy link
Contributor

For example, if we remove the method Plugin.getPluginPreferences() then all plugins that still use it will not compile anymore.

I don't see why we should replace those references, preferences are now stored according to scopes in the org.eclipse.core.runtime.preferences.IPreferencesService part of equinox and plugins should use InstanceScope.INSTANCE.getNode(yourPluginId) to set preferences and DefaultScope.INSTANCE.getNode(yourPluginId) to set default preferences.

It's my first time looking at this project, I might be misunderstaning.
Can you provide an example reference and what you would replace it with?

@laeubi
Copy link
Contributor Author

laeubi commented Jun 16, 2023

For example, if we remove the method Plugin.getPluginPreferences() then all plugins that still use it will not compile anymore.

The method has to stay, but we should remove any call to this method in platform code.

I don't see why we should replace those references, preferences are now stored according to scopes in the org.eclipse.core.runtime.preferences.IPreferencesService part of equinox and plugins should use InstanceScope.INSTANCE.getNode(yourPluginId) to set preferences and DefaultScope.INSTANCE.getNode(yourPluginId) to set default preferences.

Because the class is deprecated, that means we do not recommend to use it anymore. So it would be good to replace all usages of that class with the now recommended approach

* @deprecated This class is replaced by {@link IEclipsePreferences}. Setting a default
* value is accomplished by a setting a value in the {@link DefaultScope}, and setting
* an explicit non-default value is accomplished by setting a value in the {@link InstanceScope}.
* To obtain a preference value, use the preference accessor methods on {@link IPreferencesService}.

@lathapatil
Copy link
Contributor

This class is used in other eclipse repositories as well like in eclipse platform ui, eclipse platform text , eclipse jdt . How will this be handled ? can a single contributor work on fixing all of these ?

@akurtakov
Copy link
Member

So this is about Eclipse Platform project to stop using deprecated APIs itself. No one said anything about removing these APIs so other projects can continue using them. It would shift responisibility for fixing issues if/when they arise in usage of these APIs to whoever still uses them.

@laeubi
Copy link
Contributor Author

laeubi commented Jun 16, 2023

can a single contributor work on fixing all of these ?

Sure why not, its just a matter of time/motivation, but for sure one can simply choose one place where the deprecated class is used and replace with newer construct, so it could be iteratively done by many developers.

The goal would be that searching for references inside the eclipse-sdk will only show the class itself and public API methods that should be deprecated as well.

Once this is done, we might even mark them for removal so they can be deleted some time in the future.

@alex2145
Copy link
Contributor

I get it now, I didn't know that org.eclipse.core.runtime.Preferences was used by other modules in the project (I had imported only the runtime module and I could only find references in deprecated methods).

I imported the whole project and found more references.

@laeubi
Copy link
Contributor Author

laeubi commented Jun 17, 2023

alex2145 added a commit to alex2145/eclipse.platform that referenced this issue Jun 17, 2023
Replace deprecated preferences APIs.
In the implementation of
IPreferenceChangeListener.preferenceChange(PreferenceChangeEvent) do not
use the newValue field of the PreferenceChangeEvent object because it is
null when autobuild is being turned on (this is caused by the method
Preferences.setValue(String, boolean) removing the explicit setting when
the value is equal to the default value).

Partially fixes
eclipse-platform#497
@jukzi jukzi closed this as completed in 8b87e40 Jun 22, 2023
@laeubi laeubi reopened this Jun 22, 2023
zelenyhleb added a commit to zelenyhleb/eclipse.platform that referenced this issue Jul 1, 2023
Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
@lathapatil
Copy link
Contributor

how do we do testing for the fix we are providing for this issue ?
are there any automated tests available or we should do any manual testing before code commit?

@jukzi
Copy link
Contributor

jukzi commented Jul 11, 2023

Generally there are a lot of automated tests running before an commit is accepted and also even more tests in the day after. However it's also good to test the code before creating a PR. In the call hierarchy of the code you may eventually find a testcase that can be manually run on your local computer. Sometimes helps to debug it. If you don't find a testcase it is very appreciated to add a junit test that both validates old and new behaviour.

alex2145 added a commit to alex2145/eclipse.platform that referenced this issue Jul 13, 2023
zelenyhleb added a commit to zelenyhleb/eclipse.platform that referenced this issue Jul 14, 2023
Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
zelenyhleb added a commit to zelenyhleb/eclipse.platform that referenced this issue Jul 15, 2023
Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
zelenyhleb added a commit to zelenyhleb/eclipse.platform that referenced this issue Jul 17, 2023
Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
@jukzi jukzi closed this as completed in 009eb11 Jul 18, 2023
jukzi pushed a commit that referenced this issue Jul 31, 2023
Replace deprecated preferences APIs.

Partially fixes
#497
@lathapatil
Copy link
Contributor

Is there any reason why RefreshManager.java is not fixed for this issue and the issue is already closed?

could anyone let me know the reason before creating a PR for the same

@laeubi laeubi reopened this Aug 21, 2023
@jukzi
Copy link
Contributor

jukzi commented Aug 21, 2023

that was autoclosed by related PR.

lathapatil added a commit to lathapatil/eclipse.platform that referenced this issue Sep 18, 2023
…#497

Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
akurtakov pushed a commit to lathapatil/eclipse.platform that referenced this issue Sep 20, 2023
…#497

Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
HeikoKlare pushed a commit to lathapatil/eclipse.platform that referenced this issue Oct 2, 2023
…#497

Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
lathapatil added a commit to lathapatil/eclipse.platform that referenced this issue Oct 6, 2023
…#497

Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
lathapatil added a commit to lathapatil/eclipse.platform that referenced this issue Oct 9, 2023
…#497

Replacing deprecated Preferences API usage with modern Equinox
Preferences API.

Partially fixes
eclipse-platform#497
Updated copyright header as per review comments

Squash Commits
@vogella
Copy link
Contributor

vogella commented Oct 9, 2023

If we do not use it anymore in platform, please mark it for deletion (via the forRemoval=true attribute on the @deprecated annotation, so that we have the option to delete it in 3 years.

@HeikoKlare HeikoKlare reopened this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants