Community
Participate
Working Groups
There's a lot of deprecation warnings in PDE in regards to how we use preferences. We should review our usage and fix the deprecations to use the new preferences story.
let's look at this in M7
Ankur is going to try creating a patch.
There are many classes using the old preferences. Changing them in one shot won't be right idea. How should I segregated them? Making patches per Project/package would be OK?
small steps are fine, start with what you're comfortable with
Just put up regular patches with what you've got done. I'll try to keep up with reviewing/committing them as they come in.
Created attachment 130171 [details] Work in progress - Patch 1 Please review this patch and let me know if I am doing it right.
I am still quite confused as to what the proper preferences story is, but I do see one potential problem with your work. You are changing the preference initializer to store default values in the default scope. However, whenever you get preferences you are only grabbing them from the instance scope and providing an explicit default value. This means that the default value is likely being ignored (I haven't tested it). It would be better to use the Platform preference service getString() getBoolean() etc. methods because that will check all scopes in the default order. IPreferencesService service = Platform.getPreferencesService(); service.getString(PLUGIN_ID, prefkey, default, null);
The proper story is the one that isn't deprecated :)
(In reply to comment #8) > The proper story is the one that isn't deprecated :) > There are multiple ways to do the preferences without using deprecated code. If we are going to change all of PDE to avoid the warnings, we want to do it right, not end up with random preferences problems throughout 3.5.x. The preference service appears to be the proper way to access things, but other plug-ins are directly accessing the different scopes.
One step at a time ;) I would prefer if we would have no deprecated references by the time we ship 3.5 Since we are in the platform, we should use the IEclipsePreferences mechanism over the vanilla OSGi preferences.
so whats the final decision? IPreferencesService or IEclipsePreferences ?
IEclipsePreferences But I would like to make sure that the defaults are being picked up correctly. I will try to do some manual testing. It would be even better if we could create a set of tests that make sure that the preferences work correctly (at the project, instance and default scopes). i.e. have a set of known preferences and their expected default values, some which can be set per project, some which are instance only and make sure the stored preference value is correct.
Created attachment 130979 [details] Pref Story test cases Some test cases based on the Preferences usage in PDE. Will add more as I progress.
Created attachment 130980 [details] Pref fixes for o.e.pde.core Curtis, I've made changes to the way I am approaching this problem. Please review this and let me know your comments/suggestions. Thanks, Ankur..
Created attachment 131458 [details] Test Case Patch Added PDE specific test cases
Created attachment 131459 [details] Pref Fixes for o.e.pde.core Switched to PDEPreferencesManager approach.
Created attachment 131467 [details] Combine and updated patch Unfortunately I have a concern that needs to be addressed before putting this in. The old and new preferences do not appear to be compatable. i.e. setting preferences with the new manager does not change the values when getting from the old style preferences (and vice versa). This is a problem for two reasons, people with existing workspaces won't have their preferences migrated, and PDE UI (and possibly others, though the code is internal) still get preferences the old way and so they will be missing values. I added a simple test to the patch for this. I would also like to see the PDEPreferencesManager have javadoc.
Created attachment 131468 [details] mylyn/context/zip
The old and new preferences are compatible. I just tested. The small glitch in the test case was that the PDECore.getDefault().getPluginPreferences() gets the o.e.pde.core 's preferences. While the "stringKey" exists in the test case plug-in (private static final String PLUGIN_ID = "org.eclipse.pde.ui.tests";) I have changes the test case to assert equals on prefs fetched by old and new preference mechanism.
Made a couple more changes, fixed up the tests and committed the fix to HEAD.
Created attachment 131545 [details] o.e.pde.ui pref fixes The o.e.pde.ui switched to PDEPreferencesManager. The usage of IPreferenceStore has not be touched as it hasn't deprecated. Test cases have been added that compare the compatibility of 1) PDEPreferencesManager and Preferences 2) PDEPreferencesManager and IPreferenceStore
Applied the UI patch. Added bug 272021 to finish documenting the manager. Thanks Ankur.
